Skip to content

Do not keep the build directory of the pinned packages#6436

Merged
rjbou merged 2 commits into
ocaml:masterfrom
kit-ty-kate:no-keep-build-dir-pinned
Apr 4, 2025
Merged

Do not keep the build directory of the pinned packages#6436
rjbou merged 2 commits into
ocaml:masterfrom
kit-ty-kate:no-keep-build-dir-pinned

Conversation

@kit-ty-kate

Copy link
Copy Markdown
Member

Fixes #4255 for good
Partially f.i.x.e.s #5448

Reasoning:

The build directory is cleared anyway before each builds and is available in sources already so keeping it around doesn't have any purpose and only wastes precious disk space.

@kit-ty-kate kit-ty-kate force-pushed the no-keep-build-dir-pinned branch from 2716628 to fe1a71d Compare March 27, 2025 16:11
@kit-ty-kate kit-ty-kate requested a review from rjbou March 27, 2025 16:11
@kit-ty-kate kit-ty-kate added this to the 2.4.0~alpha1 milestone Mar 31, 2025

@rjbou rjbou left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I've exported the test to its own test file, to add more test for the OPAMKEEPBUILDDIR environment variable. It may worth to add some test with --keep-build-dir?

Comment thread src/client/opamSolution.ml
@rjbou rjbou force-pushed the no-keep-build-dir-pinned branch from fe1a71d to 9c53c3b Compare April 3, 2025 13:31
@kit-ty-kate kit-ty-kate force-pushed the no-keep-build-dir-pinned branch from 9c53c3b to d63df08 Compare April 3, 2025 15:14
@kit-ty-kate

Copy link
Copy Markdown
Member Author

It may worth to add some test with --keep-build-dir?

done. I've also extended your new reftest with the missing "repository package with local and vcs sources" cases

@kit-ty-kate kit-ty-kate requested a review from rjbou April 3, 2025 15:16
@kit-ty-kate kit-ty-kate force-pushed the no-keep-build-dir-pinned branch from d63df08 to 47962aa Compare April 4, 2025 13:39
@kit-ty-kate kit-ty-kate force-pushed the no-keep-build-dir-pinned branch from 47962aa to 047c2cd Compare April 4, 2025 14:19

@rjbou rjbou left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Build directories kept again for pinned packages

2 participants