Skip to content
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

reuse config/build_config.txt for all bootstrap scripts (posix + windows + ci); use build_all.bat in 1 CI, fix bug in build_all.bat #17899

Merged
merged 8 commits into from
May 1, 2021

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Apr 30, 2021

  • makes sure build_all.bat keeps working in CI by bootstrapping from it instead of nimBuildCsourcesIfNeeded in 1 CI (but nimBuildCsourcesIfNeeded works cross-platform and CI tests for this already)
  • factor the csources_v1's url + hash so they only need to be defined once
  • case in point justifying this PR: fixes a pre-existing bug in build_all.bat:
    PROCESSOR_ARCHITECTURE was silently ignored, because code was using if PROCESSOR_ARCHITECTURE=="AMD64" instead of if "%PROCESSOR_ARCHITECTURE%"=="AMD64"

future work

@timotheecour timotheecour changed the title reuse config/build_config.txt for all bootstrap scripts (posix + windows + ci); use build_all.bat in 1 CI reuse config/build_config.txt for all bootstrap scripts (posix + windows + ci); use build_all.bat in 1 CI, fix bug in build_all.bat Apr 30, 2021
@timotheecour timotheecour force-pushed the pr_build_all_bat_refactor branch 2 times, most recently from bc73659 to 24adc2d Compare April 30, 2021 06:14
@timotheecour timotheecour marked this pull request as ready for review April 30, 2021 07:34
@timotheecour timotheecour added the Ready For Review (please take another look): ready for next review round label Apr 30, 2021
build_all.bat Outdated Show resolved Hide resolved
build_all.bat Outdated Show resolved Hide resolved
@timotheecour timotheecour force-pushed the pr_build_all_bat_refactor branch from 36b769f to 76ce95e Compare April 30, 2021 20:43

rem Read in some common shared variables (shared with other tools),
rem see https://stackoverflow.com/questions/3068929/how-to-read-file-contents-into-a-variable-in-a-batch-file
for /f "delims== tokens=1,2" %%G in (config/build_config.txt) do set %%G=%%H
Copy link
Member

Choose a reason for hiding this comment

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

Maybe later: ci_generate.nim should generate these pairs from build_config.txt so that we don't depend on obscure Batch features.

@Araq Araq merged commit 1f1d85b into nim-lang:devel May 1, 2021
@timotheecour timotheecour deleted the pr_build_all_bat_refactor branch May 1, 2021 05:35
@timotheecour timotheecour removed the Ready For Review (please take another look): ready for next review round label May 1, 2021
PMunch pushed a commit to PMunch/Nim that referenced this pull request Mar 28, 2022
…ows + ci); use build_all.bat in 1 CI, fix bug in build_all.bat (nim-lang#17899)

* reuse config/build_config.txt for all bootstrap scripts (posix + windows + ci)
* ci_docs: use build_all.bat in CI (just in that pipeline) to ensure it keeps working
* fixup
* fix pre-existing bug in build_all.bat
* fixup
* cp => copy /y
* auto-generate build_all.bat, build_all.sh
* fixup
Araq pushed a commit that referenced this pull request Oct 14, 2024
narimiran pushed a commit that referenced this pull request Jan 14, 2025
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.

2 participants