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

Automate artefact collection; build for all VS & clang versions #18

Merged
merged 22 commits into from
Apr 13, 2023

Conversation

h-vetinari
Copy link
Member

@h-vetinari h-vetinari commented Mar 18, 2023

So far, I've just been updating this recipe from the clang side, but I realize now that it's still depending on VS2017, and hasn't yet switched to the new VS2019 default.

But I realised that to be fully usable, this recipe needs to be built for all our available VS versions (as well as all our live clang versions), but that becomes very unwieldy and tiresome with all the hardcoded components/urls.

So I automated this and then expanded the build matrix. I also added myself to maintainers now.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • It looks like the 'winsdk' output doesn't have any tests.
  • It looks like the 'msvc-headers-libs' output doesn't have any tests.

@h-vetinari
Copy link
Member Author

(side note - wouldn't we want to build this for VS2019/VS2022 in parallel, like on the vc-feedstock? Or maybe even in the vc-feedstock, if we need to pin so hard to vc- resp. ucrt- versions?)

I'm thinking we should keep this here but perhaps build for the 2x2 matrix of supported (= two latest) VS resp. clang versions?

.ci_support/linux_64_.yaml Outdated Show resolved Hide resolved
@h-vetinari h-vetinari force-pushed the vs branch 5 times, most recently from 0aebc09 to c4c967b Compare April 10, 2023 06:02
@h-vetinari h-vetinari changed the title Update to VS2019 Automate artefact collection; build for all VS versions Apr 10, 2023
@h-vetinari h-vetinari force-pushed the vs branch 8 times, most recently from 0c8f630 to 551a96d Compare April 10, 2023 07:46
@h-vetinari h-vetinari changed the title Automate artefact collection; build for all VS versions Automate artefact collection; build for all VS & clang versions Apr 10, 2023
@h-vetinari h-vetinari force-pushed the vs branch 7 times, most recently from 3c7b738 to 80b1d02 Compare April 10, 2023 08:09
@h-vetinari
Copy link
Member Author

h-vetinari commented Apr 10, 2023

@xhochy @isuruf
After much trial and error, I managed to automate the artefact collection here and generalize it to all VS-versions. The commits are cleaned up to be reviewable individually, though the "meat" of the matter is 10a7739.

I've tried to document my understanding of where the various versions come from, and I'd very much appreciate your review on this, as well as the automation of course (though the fact that it works should hopefully speak for itself). Importantly, I'm still inserting hard-coded links into the activation scripts (at installation time), so in principle they run exactly as before.

As far as I understand it, for full functionality we actually need the whole matrix of VS-versions times clang-versions that we have live in conda-forge; I've built this up incrementally: first automate only VS2017, then expand the VS-versions in CBC, then expand the clang matrix as well. You can see that the CI for all these stages is passing -- obviously we have a choice to reduce the matrix as desired.

PTAL

Comment on lines +28 to +31
# component=$(echo $map | cut -d '|' -f1)
url=$(echo $map | cut -d '|' -f2)
# download and unpacks into Contents/VC/Tools/MSVC/${MSVC_HEADERS_VERSION}/...
curl -L $url -o tmp.zip && unzip -oqq tmp.zip && rm tmp.zip
Copy link
Member Author

Choose a reason for hiding this comment

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

After I had scripted all the automation, I ended up not using the component name explicitly in the activation script. I still think it's a value add to keep it, as that makes the activation script way more self-explanatory with the component names in COMPONENT_URLS, than if it were just a list of obfuscated URLs.

recipe/install-pkg.sh Outdated Show resolved Hide resolved
sed-replace directly with ${CHOST_BASE}${CL_VERSION} in install-pkg.sh
recipe/install-pkg.sh Outdated Show resolved Hide resolved
@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • The top level meta keys are in an unexpected order. Expecting ['package', 'source', 'build', 'outputs', 'about', 'extra'].

For recipe:

  • It looks like the 'winsdk' output doesn't have any tests.
  • It looks like the 'msvc-headers-libs' output doesn't have any tests.

recipe/meta.yaml Outdated Show resolved Hide resolved
@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • It looks like the 'winsdk' output doesn't have any tests.
  • It looks like the 'msvc-headers-libs' output doesn't have any tests.

@isuruf
Copy link
Member

isuruf commented Apr 13, 2023

Thanks

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.

3 participants