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

Fix overlapping file lock exception #5489

Merged
merged 4 commits into from
Nov 11, 2024

Conversation

jorgee
Copy link
Contributor

@jorgee jorgee commented Nov 8, 2024

Fix conda packages with same name and content but different path.
Include a hash of the path in the environment prefix directory.

@jorgee jorgee linked an issue Nov 8, 2024 that may be closed by this pull request
@jorgee jorgee marked this pull request as draft November 8, 2024 14:12
Copy link

netlify bot commented Nov 8, 2024

Deploy Preview for nextflow-docs-staging ready!

Name Link
🔨 Latest commit 7edc0e7
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/6730ef4af448fc00083b1a62
😎 Deploy Preview https://deploy-preview-5489--nextflow-docs-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@jorgee jorgee marked this pull request as ready for review November 8, 2024 15:22
@ewels
Copy link
Member

ewels commented Nov 8, 2024

Tested locally with this:

nextflow run nf-core/rnaseq -r 3.17.0 -profile test,conda --outdir dir

Breaks on the released version and works fine with this PR build 🎉

Copy link
Member

@ewels ewels left a comment

Choose a reason for hiding this comment

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

If we could get this into a patch release that would be amazing 🙏🏻

@pditommaso
Copy link
Member

Made a change to use normalised absolute path + using more compact sip hash algorithm.

How this solution is going to be validated?

@jorgee
Copy link
Contributor Author

jorgee commented Nov 11, 2024

Made a change to use normalised absolute path + using more compact sip hash algorithm.

How this solution is going to be validated?

I could try to add an integration tests that installs miniconda and runs the pipeline which reproduces the error. Do you think it is enough?

@pditommaso
Copy link
Member

I was more thinking running a real pipeline to replicate the problem. Maybe @ewels knows more

@ewels
Copy link
Member

ewels commented Nov 11, 2024

I was trying to validate earlier this morning already, but couldn't get the original error to trigger any more 🙈

I'll clean up my env and try again shortly. Maybe @maxulysse or @pinin4fjords could also have a go?

@maxulysse
Copy link
Contributor

I was trying to validate earlier this morning already, but couldn't get the original error to trigger any more 🙈

I'll clean up my env and try again shortly. Maybe @maxulysse or @pinin4fjords could also have a go?

I can definitively replicate the original error

@pditommaso
Copy link
Member

@maxulysse nice. Are you able to make a local build with this branch and try it?

@maxulysse
Copy link
Contributor

@maxulysse nice. Are you able to make a local build with this branch and try it?

It's a success 🚀

@pditommaso
Copy link
Member

Great!

@pditommaso pditommaso merged commit eaaeb3d into master Nov 11, 2024
10 checks passed
@pditommaso pditommaso deleted the 5485-conda-overlappingfilelockexception branch November 11, 2024 11:19
@ewels
Copy link
Member

ewels commented Nov 11, 2024

I was too slow, but I just checked on GitPod and was able to replicate the error and confirm that it's now fixed on master 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conda: OverlappingFileLockException
5 participants