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.

Sign up for GitHub

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

Jump to bottom

support stdlib() jinja function #4999

Merged
merged 14 commits into from
Oct 25, 2023
Merged

support stdlib() jinja function #4999

merged 14 commits into from
Oct 25, 2023

Conversation

h-vetinari
Copy link
Contributor

@h-vetinari h-vetinari commented Sep 10, 2023 •
edited
Loading

Description

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

Fixes #4981

Continuation of #4996, as requested.

cc @chenghlee, @mbargull, @beckermr

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Sep 10, 2023
@h-vetinari h-vetinari mentioned this pull request Sep 10, 2023
Closed
3 tasks
@isuruf
Copy link
Contributor

isuruf commented Sep 24, 2023

Let's add a test and get this in.

@h-vetinari
Copy link
Contributor Author

Test added; I tried to copy an existing test for the compiler-jinja. Could someone trigger the tests please?

@h-vetinari
Copy link
Contributor Author

I'm not sure if GHA is having problems or if it's a rights issue, but I cannot inspect any of the failing (or passing) test runs - the logs straight up fail to load (raw logs too). I'll try to have a look again tomorrow.

isuruf
isuruf reviewed Sep 24, 2023
View reviewed changes
tests/test_metadata.py Outdated Show resolved Hide resolved
@h-vetinari
Copy link
Contributor Author

Thanks for the fixes, now I can also see the test results. :)

@h-vetinari
Copy link
Contributor Author

Cannot inspect the failing test runs, again... 😑

@h-vetinari h-vetinari mentioned this pull request Sep 26, 2023
Closed
9 tasks
@h-vetinari
Copy link
Contributor Author

Finally was able to inspect the logs, and all the failures are either flaky or unrelated to this PR.

Could someone take a look perhaps? 🙃
@jezdez @kenodegard @beeankha @dholth

@h-vetinari
Copy link
Contributor Author

Just saw #5013; presumably we're too late to catch that release-train? 😑

@jezdez
Copy link
Member

jezdez commented Sep 27, 2023

@h-vetinari I'm sorry yeah, we're pretty close to the end of the month as is, but I'm sure we'll do a follow-up release! (don't have to wait for 2 months)

@isuruf isuruf requested a review from a team October 2, 2023 16:07
@JeanChristopheMorinPerso
Copy link
Contributor

Should we add docs in this PR or should it be done in a separate PR?

@dholth
Copy link
Contributor

dholth commented Oct 5, 2023

It should be the same PR.

h-vetinari
h-vetinari commented Oct 6, 2023
View reviewed changes
Copy link
Contributor Author

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

Should we add docs in this PR or should it be done in a separate PR?

It should be the same PR.

Done; I added it to the page describing {{ compiler(<lang>) }}, directly below. Happy to move it elsewhere if that's not the best place.

docs/source/resources/compiler-tools.rst Outdated
Comment on lines 446 to 450
In principle, this facility would make it possible to also express the
dependence on separate stdlib implementations (like ``musl`` instead of
``glibc``), or to remove the need to assume that a C++ compiler always needs to
add a run-export on the C++ stdlib -- it could then be left up to packages
themselves whether they need ``{{ compiler('cxx') }}`` or not.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The paragraph is a bit forward-looking in nature. It's not exactly what we need in the first iteration, but possible future uses that are enabled by it. I don't mind deleting it, but thought I'd add it for completeness.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gah, this should have said "whether they need {{ stdlib('cxx') }} or not." of course.

Callek
Callek reviewed Oct 6, 2023
View reviewed changes
Copy link
Contributor

@Callek Callek left a comment

Choose a reason for hiding this comment

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

Just was reading the docs for this and saw a something to point out

docs/source/resources/compiler-tools.rst Outdated Show resolved Hide resolved
JeanChristopheMorinPerso
JeanChristopheMorinPerso reviewed Oct 6, 2023
View reviewed changes
docs/source/resources/compiler-tools.rst Outdated Show resolved Hide resolved
docs/source/resources/compiler-tools.rst Outdated Show resolved Hide resolved
docs/source/resources/compiler-tools.rst Outdated Show resolved Hide resolved
@h-vetinari
Copy link
Contributor Author

Could someone trigger the CI workflow here (and ideally: review the PR)? It would be a pity if this missed another cycle.

@h-vetinari h-vetinari mentioned this pull request Oct 15, 2023
Merged
@jakirkham
Copy link
Member

Could someone please let us know what is still needed here?

@h-vetinari
Copy link
Contributor Author

Gentle ping here :)

@beckermr beckermr merged commit e2916a2 into conda:main Oct 25, 2023
22 checks passed
@beckermr
Copy link
Contributor

YOLO don't hate me.

@jakirkham
Copy link
Member

Thanks all! 🙏

@h-vetinari h-vetinari deleted the stdlib branch October 25, 2023 18:32
This was referenced Oct 25, 2023
Open
Closed
@jezdez jezdez added this to the 3.28.0 milestone Nov 13, 2023
@kenodegard kenodegard mentioned this pull request Dec 1, 2023
Closed
67 tasks
@h-vetinari h-vetinari mentioned this pull request Dec 12, 2023
Closed
@h-vetinari h-vetinari mentioned this pull request Feb 17, 2024
Closed
2 tasks
@h-vetinari h-vetinari mentioned this pull request Feb 28, 2024
Merged
3 tasks
@h-vetinari h-vetinari mentioned this pull request Mar 8, 2024
Open
57 tasks
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Nov 13, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JeanChristopheMorinPerso JeanChristopheMorinPerso JeanChristopheMorinPerso left review comments

@isuruf isuruf isuruf left review comments

@Callek Callek Callek left review comments

@beckermr beckermr beckermr approved these changes

@chenghlee chenghlee chenghlee approved these changes

@jaimergp jaimergp jaimergp approved these changes

Assignees
No one assigned
Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity
Projects
🔎 Review
Archived in project
Milestone
3.28.0
Development

Successfully merging this pull request may close these issues.

New jinja-function for handling sysroot