Skip to content

Conversation

@timotheecour
Copy link
Member

@timotheecour timotheecour commented Apr 21, 2021

  • follows CIs: attempt to use csources_v1 #16282, refs CIs: attempt to use csources_v1 #16282 (comment)
  • refactor all code that builds csources so that changing csources url/commit can be done in a single place (nimBuildCsourcesIfNeeded), without affecting scripts
  • updating csources_v1 won't affect nim repo since this PR hardcodes the hash at which to build csources_v1
  • sh build_all.sh now does "the right thing" transparently (caches bin/nim_csources_$hash and only rebuilds when needed)
  • remove ci/* except ci/funs.sh, which by design has no top-level statement, only reusable bash functions that can be used after . ci/funs.sh
  • simplify CI pipelines (as you can see, the diff removes code overall)
  • fixes Lock down csources version #11457

design goals

  • DRY: hide the way we build csources (bootstrap nim) behind a bash function call, nimBuildCsourcesIfNeeded
  • reusable bash functions for CI + build_all (note we still prefer nim scripts but this is for bootstrapping)
  • future proof design so that we can build nim at any revision even if csources url/hash is later updated (ie we tie a nim version to a corresponding csources version), allowing tooling eg:
    • git bisect workflows
    • running benchmarks across several nim versions
  • we won't need a new git repo csources_v2 etc, instead we can reuse csources_v1 with different commits (or tags/branches if needed for clarity)

notes

  • I haven't updated appveyor.yml.disabled, it's outdated and not used anymore
  • I haven't updated build_all.bat, it was already outdated (used csources instead of csources_v1), and I don't know if it's still used by windows users (given we have choosenim and that windows users can use bash scripts IIRC, eg as does azure_pipelines.yml); if it still needs to be maintained, we can update it (or even auto-generate it to keep the urls/hash in sync)
  • I've removed the caching logic in ci_docs.yml because it wasn't used in other pipelines, it doesn't save much CI time, and it simplifies the pipeline quite a bit; if needed i can revert that change

future work

links

@timotheecour timotheecour marked this pull request as ready for review April 22, 2021 04:01
@timotheecour timotheecour force-pushed the pr_refactor_cis branch 3 times, most recently from 62f04a0 to 051ed10 Compare April 22, 2021 19:34
@timotheecour timotheecour added the Ready For Review (please take another look): ready for next review round label Apr 22, 2021
@RSDuck
Copy link
Contributor

RSDuck commented Apr 22, 2021

I haven't updated build_all.bat, it was already outdated (used csources instead of csources_v1), and I don't know if it's still used by windows users (given we have choosenim and that windows users can use bash scripts IIRC, eg as does azure_pipelines.yml); if it still needs to be maintained, we can update it (or even auto-generate it to keep the urls/hash in sync)

I did not too long ago, because it's mentioned readme, so I would assume other people do this as well.

@timotheecour
Copy link
Member Author

well I've updated it in the meantime in a prior commit, please test build_all.bat from this PR

@RSDuck
Copy link
Contributor

RSDuck commented Apr 22, 2021

everything seems to work well. Is there anything in particular I should test?

@timotheecour
Copy link
Member Author

everything seems to work well. Is there anything in particular I should test?

that's it, thanks!

@timotheecour timotheecour added TODO: followup needed remove tag once fixed or tracked elsewhere and removed Ready For Review (please take another look): ready for next review round labels Apr 23, 2021
@Araq Araq added the merge_when_passes_CI mergeable once green label Apr 23, 2021
@Araq Araq merged commit dce0b3b into nim-lang:devel Apr 23, 2021
@timotheecour timotheecour deleted the pr_refactor_cis branch April 23, 2021 09:35
PMunch pushed a commit to PMunch/Nim that referenced this pull request Mar 28, 2022
* refactor all code that builds csources
* fixup
* nim_csourcesDir_v0 + nim_csourcesDir
* remove deprecated, unused scripts from ci/
* reuse nimCsourcesHash in ci
* simplify CI pipelines by reusing nimBuildCsourcesIfNeeded
* simplify ci_docs.yml by reusing nimBuildCsourcesIfNeeded
* cleanup
* use csources_v1 as destination dir
* fixup
* remove pushCsources
* address comment: remove build.sh support for now
* fixup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge_when_passes_CI mergeable once green TODO: followup needed remove tag once fixed or tracked elsewhere

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lock down csources version

3 participants