-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
framework: split armbian-bsp-cli into bsp-cli and bsp-generic. #9090
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
cdd4f0a
1f946e2
030d5ad
0dd6c61
63f11b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| #!/usr/bin/env bash | ||
| # | ||
| # SPDX-License-Identifier: GPL-2.0 | ||
| # | ||
| # Copyright (c) 2013-2023 Igor Pecovnik, [email protected] | ||
| # | ||
| # This file is a part of the Armbian Build Framework | ||
| # https://github.com/armbian/build/ | ||
|
|
||
| function artifact_armbian-bsp-generic_config_dump() { | ||
| artifact_input_variables[RELEASE]="${RELEASE}" | ||
| artifact_input_variables[BRANCH]="${BRANCH}" | ||
| } | ||
|
|
||
| function artifact_armbian-bsp-generic_prepare_version() { | ||
| : "${BRANCH:?BRANCH is not set}" | ||
|
|
||
| artifact_version="undetermined" # outer scope | ||
| artifact_version_reason="undetermined" # outer scope | ||
|
|
||
| declare short_hash_size=4 | ||
|
|
||
| declare fake_unchanging_base_version="1" | ||
|
|
||
| # Generic package has no board/family hooks | ||
| declare hash_hooks_short="0000" | ||
|
|
||
| # Hash variables that affect the contents of bsp-generic package. | ||
| # Those contain /armbian a lot, so don't normalize them. | ||
| declare -a vars_to_hash_no_normalize=() | ||
| declare hash_variables="undetermined" # will be set by calculate_hash_for_variables(), but without normalization | ||
| do_normalize_src_path="no" calculate_hash_for_variables "${vars_to_hash_no_normalize[@]}" # don't normalize | ||
| declare hash_vars_no_normalize="${hash_variables}" | ||
|
|
||
| declare -a vars_to_hash=( | ||
| "KEEP_ORIGINAL_OS_RELEASE: ${KEEP_ORIGINAL_OS_RELEASE:-"no"}" # /etc/os-release | ||
| "IMAGE_TYPE: ${IMAGE_TYPE}" # /etc/armbian-release | ||
| "hash_vars_no_normalize: ${hash_vars_no_normalize}" # The non-normalized part, above | ||
| ) | ||
| declare hash_variables="undetermined" # will be set by calculate_hash_for_variables(), which normalizes the input | ||
| calculate_hash_for_variables "${vars_to_hash[@]}" | ||
| declare vars_config_hash="${hash_variables}" | ||
| declare var_config_hash_short="${vars_config_hash:0:${short_hash_size}}" | ||
|
Comment on lines
+35
to
+43
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: find lib packages -name "*artifact-armbian-bsp-generic*" -o -name "*bsp-cli-deb*" | head -20Repository: armbian/build Length of output: 153 🏁 Script executed: cat -n lib/functions/artifacts/artifact-armbian-bsp-generic.sh | head -80Repository: armbian/build Length of output: 3929 🏁 Script executed: # Search for KEEP_ORIGINAL_OS_RELEASE usage in the codebase
rg -n "KEEP_ORIGINAL_OS_RELEASE" lib packages --type sh | head -30Repository: armbian/build Length of output: 662 🏁 Script executed: # Check what KEEP_ORIGINAL_OS_RELEASE does in bsp-cli-deb.sh
sed -n '385,395p' lib/functions/bsp/armbian-bsp-cli-deb.shRepository: armbian/build Length of output: 834 🏁 Script executed: # Search for IMAGE_TYPE in bsp-cli-deb.sh
rg -n "IMAGE_TYPE" lib/functions/bsp/armbian-bsp-cli-deb.shRepository: armbian/build Length of output: 128 🏁 Script executed: # Check what happens with IMAGE_TYPE in bsp-cli-deb.sh context
sed -n '110,130p' lib/functions/bsp/armbian-bsp-cli-deb.shRepository: armbian/build Length of output: 723 🏁 Script executed: # Check the broader context of these variables in bsp-cli-deb.sh
sed -n '100,180p' lib/functions/bsp/armbian-bsp-cli-deb.shRepository: armbian/build Length of output: 3372 🏁 Script executed: # Check if IMAGE_TYPE affects content in the directories used by generic package
rg -r "IMAGE_TYPE" packages/bsp/common/ config/optional/_any_board/_packages/bsp-generic/ --type sh 2>/dev/null | head -20Repository: armbian/build Length of output: 39 Remove IMAGE_TYPE from generic BSP package hash — it doesn't affect package content. Line 37 includes 🤖 Prompt for AI Agents
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: Length of output: 1678 🏁 Script executed: Length of output: 1070 🏁 Script executed: Length of output: 2326 🏁 Script executed: Length of output: 1351 @tabrisnet
Its primary purposes are:
The question from the previous review comment is whether (Note: Don't confuse this with 🧠 Learnings used |
||
|
|
||
| declare -a dirs_to_hash=( | ||
| "${SRC}/packages/bsp/common" # common stuff | ||
| "${SRC}/config/optional/_any_board/_packages/bsp-generic" | ||
| ) | ||
| declare hash_files="undetermined" | ||
| calculate_hash_for_all_files_in_dirs "${dirs_to_hash[@]}" | ||
| packages_config_hash="${hash_files}" | ||
| declare packages_config_hash_short="${packages_config_hash:0:${short_hash_size}}" | ||
|
|
||
| # get the hashes of the lib/ bash sources involved... | ||
| declare hash_files="undetermined" | ||
| #FIXME: do we need to fork bsp/armbian-bsp-cli-deb.sh ? | ||
| calculate_hash_for_bash_deb_artifact "bsp/armbian-bsp-cli-deb.sh" "bsp/utils-bsp.sh" | ||
| declare bash_hash="${hash_files}" | ||
| declare bash_hash_short="${bash_hash:0:${short_hash_size}}" | ||
|
|
||
| # outer scope | ||
| artifact_version="${fake_unchanging_base_version}-PC${packages_config_hash_short}-V${var_config_hash_short}-H${hash_hooks_short}-B${bash_hash_short}" | ||
|
|
||
| declare -a reasons=( | ||
| "Armbian package armbian-bsp-generic" | ||
| "BRANCH \"${BRANCH}\"" | ||
| "Packages and config files hash \"${packages_config_hash}\"" | ||
| "Hooks hash \"${hash_hooks}\"" | ||
| "Variables/bootscripts hash \"${vars_config_hash}\"" | ||
| "framework bash hash \"${bash_hash}\"" | ||
| ) | ||
|
|
||
| artifact_version_reason="${reasons[*]}" # outer scope | ||
|
|
||
| artifact_deb_repo="global" # "global" meaning: release-independent repo. could be '${RELEASE}' for a release-specific package. | ||
| artifact_deb_arch="all" # arch-specific package, or 'all' for arch-independent package. | ||
| #FIXME: BRANCH for sure, RELEASE maybe? | ||
| artifact_name="armbian-bsp-generic-${BRANCH}" | ||
| artifact_type="deb-tar" | ||
|
|
||
| artifact_map_packages=(["armbian-bsp-generic"]="${artifact_name}") | ||
|
|
||
| # Register the function used to re-version the _contents_ of the bsp-cli deb file (non-transitional) | ||
| # FIXME: can we live without this? | ||
| #artifact_debs_reversion_functions+=("reversion_armbian-bsp-generic_deb_contents") | ||
|
|
||
| # there is no transitional package, this is just boilerplate | ||
| #if artifact_armbian-bsp-generic_needs_transitional_package; then | ||
| # artifact_map_packages+=(["armbian-bsp-generic-transitional"]="armbian-bsp-generic-${BOARD}${EXTRA_BSP_NAME}") | ||
| # # Register the function used to re-version the _contents_ of the bsp-cli deb file (transitional) | ||
| # artifact_debs_reversion_functions+=("reversion_armbian-bsp-generic-transitional_deb_contents") | ||
| #fi | ||
|
|
||
| return 0 | ||
| } | ||
|
|
||
| function artifact_armbian-bsp-generic_build_from_sources() { | ||
| LOG_SECTION="compile_armbian-bsp-generic" do_with_logging compile_armbian-bsp-generic | ||
|
|
||
| # Generate transitional package when needed. | ||
| if artifact_armbian-bsp-generic_needs_transitional_package; then | ||
| : # we don't have this hook, it's just boilerplate | ||
| #LOG_SECTION="compile_armbian-bsp-generic" do_with_logging compile_armbian-bsp-generic-transitional | ||
| fi | ||
| } | ||
|
|
||
| function artifact_armbian-bsp-generic_cli_adapter_pre_run() { | ||
| declare -g ARMBIAN_COMMAND_REQUIRE_BASIC_DEPS="yes" # Require prepare_host_basic to run before the command. | ||
|
|
||
| # "gimme root on a Linux machine" | ||
| cli_standard_relaunch_docker_or_sudo | ||
| } | ||
|
|
||
| function artifact_armbian-bsp-generic_cli_adapter_config_prep() { | ||
| # there is no need for aggregation here. | ||
| use_board="no" allow_no_family="yes" skip_kernel="no" prep_conf_main_minimal_ni < /dev/null # no stdin for this, so it bombs if tries to be interactive. | ||
| } | ||
|
|
||
| function artifact_armbian-bsp-generic_get_default_oci_target() { | ||
| artifact_oci_target_base="${GHCR_SOURCE}/armbian/os/" | ||
| } | ||
|
|
||
| function artifact_armbian-bsp-generic_is_available_in_local_cache() { | ||
| is_artifact_available_in_local_cache | ||
| } | ||
|
|
||
| function artifact_armbian-bsp-generic_is_available_in_remote_cache() { | ||
| is_artifact_available_in_remote_cache | ||
| } | ||
|
|
||
| function artifact_armbian-bsp-generic_obtain_from_remote_cache() { | ||
| obtain_artifact_from_remote_cache | ||
| } | ||
|
|
||
| function artifact_armbian-bsp-generic_deploy_to_remote_cache() { | ||
| upload_artifact_to_oci | ||
| } | ||
|
|
||
| function artifact_armbian-bsp-generic_needs_transitional_package() { | ||
| return 1 # we're too new to need a transitional package. Note that this follows the C/bash errno convention where 0 is true | ||
| # see equivalent for artifact_armbian-bsp-cli | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: armbian/build
Length of output: 5015
🏁 Script executed:
Repository: armbian/build
Length of output: 39
🏁 Script executed:
Repository: armbian/build
Length of output: 368
🏁 Script executed:
Repository: armbian/build
Length of output: 638
🏁 Script executed:
Repository: armbian/build
Length of output: 392
🏁 Script executed:
# Compare with release-specific artifact to understand expected pattern head -30 lib/functions/artifacts/artifact-armbian-base-files.shRepository: armbian/build
Length of output: 1305
Clarify RELEASE usage or remove it from config_dump.
RELEASEis captured as an artifact input variable (line 11) but is never validated (line 16 only checksBRANCH), never included in the version hash (lines 35-43), and never used in the artifact name (line 78). The package is explicitly marked as release-independent (artifact_deb_repo="global"on line 75), and a FIXME comment on line 77 questions whether RELEASE should be included at all ("BRANCH for sure, RELEASE maybe?").This creates an inconsistency between what's declared as input and what actually affects the artifact. If the generic BSP package should be release-independent, remove
RELEASEfromconfig_dump. If it should be release-specific, add validation and include it in the version calculation (as done in artifact-armbian-base-files.sh).🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not yet clear if it should be release specific or not. waiting on more knowledgeable heads
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tabrisnet, thanks for clarifying! That makes sense given the PR is still under construction. The existing FIXME comment on line 77 ("BRANCH for sure, RELEASE maybe?") already documents this uncertainty nicely.
Once the maintainers provide guidance on whether armbian-bsp-generic should be release-specific or release-independent, feel free to tag me and I can help ensure the implementation is consistent with whichever approach is chosen (updating validation, version hash calculation, and artifact naming accordingly).
🧠 Learnings used