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

Don't check build for circular dependencies when cross compiling #4248

Merged
merged 3 commits into from
Sep 1, 2021
Merged

Don't check build for circular dependencies when cross compiling #4248

merged 3 commits into from
Sep 1, 2021

Conversation

isuruf
Copy link
Contributor

@isuruf isuruf commented Jul 15, 2021

When cross compiling, build dependencies are coming from already
built packages and cannot be subpackages from the same recipe.

This is similar to #4011

@anaconda-issue-bot anaconda-issue-bot added the cla-signed [bot] added once the contributor has signed the CLA label Jul 15, 2021
@isuruf isuruf marked this pull request as draft July 15, 2021 18:13
@isuruf
Copy link
Contributor Author

isuruf commented Jul 15, 2021

nvm, this doesn't work.

@isuruf isuruf closed this Jul 15, 2021
@isuruf isuruf reopened this Jul 16, 2021
@isuruf
Copy link
Contributor Author

isuruf commented Jul 16, 2021

It does work.

@isuruf isuruf marked this pull request as ready for review July 16, 2021 17:20
@isuruf
Copy link
Contributor Author

isuruf commented Jul 16, 2021

I didn't find any tests where Circular dependencies in recipe exception is triggered and caught.

@isuruf
Don't check build for circular dependencies when cross compiling
cbd0bb2 
When cross compiling, build dependencies are coming from already
built packages and cannot be subpackages from the same recipe.
isuruf added a commit to isuruf/ctng-compilers-feedstock that referenced this pull request Jul 17, 2021
@isuruf
hack for conda/conda-build#4248
5137ae1
@ocefpaf
Copy link
Contributor

ocefpaf commented Jul 22, 2021

@chenghlee and @jezdez this work is part of Isuru's Small Grant Development proposal to NumFOCUS. Do you mind taking a look at it?

@jezdez
Copy link
Member

jezdez commented Jul 22, 2021

@ocefpaf Sure, but it would really be better to have tests for this change since in isolation I can only guess if this is a good idea or not. Do you think you could write some @isuruf?

@isuruf
Copy link
Contributor Author

isuruf commented Jul 23, 2021

I added a test. I couldn't find any existing test that checks for failure with circular dependencies, so not sure if this is what you are looking for.

@isuruf
Copy link
Contributor Author

isuruf commented Aug 11, 2021

Ping on this

@jezdez jezdez requested a review from a team August 26, 2021 14:01
@isuruf isuruf mentioned this pull request Sep 1, 2021
Merged
mbargull
mbargull approved these changes Sep 1, 2021
View reviewed changes
Copy link
Member

@mbargull mbargull left a comment

Choose a reason for hiding this comment

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

Code changes LGTM (+verified that the added test_circular_deps_cross fails without the changes), thanks!
Could you add a news entry?

@mbargull mbargull merged commit 13aa64b into conda:master Sep 1, 2021
@mbargull
Copy link
Member

mbargull commented Sep 1, 2021

Thanks again!

@isuruf isuruf deleted the cross branch September 1, 2021 20:33
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Sep 2, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mbargull mbargull mbargull 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
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@isuruf @ocefpaf @jezdez @mbargull @anaconda-issue-bot