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

ENH check all sysroot locations for DSOs #3969

Merged
merged 2 commits into from
Jun 30, 2020
Merged

ENH check all sysroot locations for DSOs #3969

merged 2 commits into from
Jun 30, 2020

Conversation

beckermr
Copy link
Contributor

@beckermr beckermr commented Jun 9, 2020

This PR changes the DSO checking code to examine all sysroots for potential DSOs when trying to detect missing DSOs in the post-build stage.

We (@isuruf and myself), found that when we have a symlink to the sysroot in the build env and we refer to a DSO by its symlinked location, conda build thinks it's missing because it only checks one spot.

The fact that it only checks one has been in the code since its start in these commits

IDK why it only checks one. @mingwandroid should comment on that.

@cla-bot cla-bot bot added the cla-signed [bot] added once the contributor has signed the CLA label Jun 9, 2020
@beckermr
Copy link
Contributor Author

beckermr commented Jun 9, 2020

cc @jjhelmus

@beckermr
Copy link
Contributor Author

beckermr commented Jun 9, 2020

If this change is accepted, I'd very much like it to make the next release.

@beckermr
Copy link
Contributor Author

beckermr commented Jun 9, 2020

Also, we need this as part of shipping cos7 on conda-forge.

@beckermr
Copy link
Contributor Author

Bump here everyone.

@beckermr
Copy link
Contributor Author

another bump here

@beckermr
Copy link
Contributor Author

bump again on this

cc @mingwandroid @jjhelmus

@mingwandroid
Copy link
Contributor

Should we do a rebuild of all cos6 cdts with this fix? And host them on CF? I'm happy with that but will also upload them to defaults.

@beckermr
Copy link
Contributor Author

Yes this is my plan.

@beckermr
Copy link
Contributor Author

Shall we merge @mingwandroid ?

@beckermr
Copy link
Contributor Author

bump here again on merging

@mingwandroid
Copy link
Contributor

Thanks @beckermr!

@beckermr
Copy link
Contributor Author

You closed it but didn't merge it @mingwandroid. :)

@beckermr
Copy link
Contributor Author

Bump here @mingwandroid @jjhelmus this was accidentally closed instead of merged. Can we please reopen and merge it?

1 similar comment
@beckermr
Copy link
Contributor Author

Bump here @mingwandroid @jjhelmus this was accidentally closed instead of merged. Can we please reopen and merge it?

@beckermr
Copy link
Contributor Author

Thanks @marcelotrevisani! Can you please merge this one?

cc @jjhelmus @mingwandroid

@marcelotrevisani
Copy link
Member

I would be more comfortable if you create a test to cover this scenario.
Could you please add a test for it?

@beckermr
Copy link
Contributor Author

@mingwandroid was happy to merge without. I’ve tested the code live.

Making a test for this is pretty hard as id need a package to build, link to some dso and then insert two sysroot paths in the env to find the dso in.

@beckermr
Copy link
Contributor Author

Bump here again. Tests for this edge case are quite difficult. I’d like to merge without them.

Can we please merge this?

@beckermr
Copy link
Contributor Author

bump here again @jjhelmus @mingwandroid

@beckermr
Copy link
Contributor Author

Ugh now there are conflicts

@mingwandroid mingwandroid merged commit 8736bcc into conda:master Jun 30, 2020
@mingwandroid
Copy link
Contributor

Oops, sorry!

@beckermr
Copy link
Contributor Author

Thanks!

@github-actions
Copy link

Hi there, thank you for your contribution!

This pull request has been automatically locked because it has not had recent activity after being closed.

Please open a new issue or pull request if needed.

Thanks!

@github-actions github-actions bot added the locked [bot] locked due to inactivity label Mar 11, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants