Skip to content

Use script for config downloads#138

Merged
TheMrIron2 merged 2 commits into
masterfrom
config-refactor
Jun 8, 2026
Merged

Use script for config downloads#138
TheMrIron2 merged 2 commits into
masterfrom
config-refactor

Conversation

@TheMrIron2
Copy link
Copy Markdown
Contributor

@TheMrIron2 TheMrIron2 commented Jun 7, 2026

Per previous PR #137 and branch config-refactor, use get-config-scripts.sh to get configs instead of manually wget from Savannah. Ensures overarching logic matches and has local fallbacks if Savannah is unreachable.

Cleaned up the noise and removed any orthogonal elements.

Per previous PR and branch, use `get-config-scripts.sh` to get configs instead of manually `wget` from Savannah. Ensures overarching logic matches and has local fallbacks if Savannah is unreachable.
@TheMrIron2 TheMrIron2 requested review from bucanero and zeldin June 7, 2026 13:04
@zeldin
Copy link
Copy Markdown
Member

zeldin commented Jun 7, 2026

LGTM. But just so you know: By placing get_config_scripts.sh in the scripts/ directory, you make it a "build script". Which means that running toolchain.sh without arguments will add this script to the list of things to run. But because of lexical ordering, it will be run after all other scripts, where it can do no good (although it will do no harm either).

@TheMrIron2
Copy link
Copy Markdown
Contributor Author

I think we ought to move it somewhere more appropriate in that case. Beside the local copies of config, or do you have another suggestion?

@zeldin
Copy link
Copy Markdown
Member

zeldin commented Jun 7, 2026

Well, having the three files in the same dir is fine, but then I don't think it should be called assets. Something like tools or utils would probably be appropriate for the script, and maybe config if you want a dir for all three files.

chmod +x "${file}"
}

fetch_config_file config.guess
Copy link
Copy Markdown

@clienthax clienthax Jun 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please wrap these for CI purposes

if [ -z "${NO_SAVANNAH}" ]; then
    fetch_config_file config.guess
    fetch_config_file config.sub
else
    echo "NO_SAVANNAH is set, skipping Savannah downloads."
    cp "${ASSET_DIR}/config.guess" "config.guess"
    cp "${ASSET_DIR}/config.sub" "config.sub"
fi

Rename `assets` directory to `config` and contained within this, the GNU `config` files and our helper script. Done per feedback to avoid this running at the end of regular execution.
@TheMrIron2
Copy link
Copy Markdown
Contributor Author

@zeldin Agreed with the suggestion. They're now grouped in the config directory.
I can add the NO_SAVANNAH flag if we agree on it, but I decided against incorporating it as part of the expected changes.

@TheMrIron2 TheMrIron2 marked this pull request as ready for review June 7, 2026 22:02
@TheMrIron2 TheMrIron2 merged commit a6115ab into master Jun 8, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants