Skip to content

Disable git gc/maintenance on repositories opam maintains - #7073

Merged
kit-ty-kate merged 1 commit into
ocaml:masterfrom
kit-ty-kate:no-git-maintenance
Aug 20, 2026
Merged

Disable git gc/maintenance on repositories opam maintains#7073
kit-ty-kate merged 1 commit into
ocaml:masterfrom
kit-ty-kate:no-git-maintenance

Conversation

@kit-ty-kate

@kit-ty-kate kit-ty-kate commented Aug 11, 2026

Copy link
Copy Markdown
Member

Partially f.i.x.e.s #7031

This should fix most sources of errors when using recent git versions, however this won't help users avoid this type of error during things like opam pin add -k path . of a git repository (maybe --workingdir although this one i'm not sure) or any copy or directory non-git aware scans of things that contain git repositories.

This will also not fix already created repositories. However an alternative could be to use GIT_CONFIG_KEY_* and GIT_CONFIG_VALUE_* environment variable instead. Doing it this way would fix it retroactively as well. It might also help for things like #6955 if we generalise the use of these variables.
EDIT: see fixup commit

A downside of disabling these detachable jobs in theory is that we could loose efficiency by not repacking once in a while but i think this should be fine.

@kit-ty-kate kit-ty-kate added this to the 2.6.0~alpha2 milestone Aug 11, 2026
@kit-ty-kate
kit-ty-kate force-pushed the no-git-maintenance branch 2 times, most recently from e8a5b14 to a72c2f1 Compare August 13, 2026 19:11

@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.

On the idea, lgtm! Somme comments to address and good to go

Comment thread src/repository/opamGit.ml Outdated
Comment thread src/core/opamStd.mli Outdated
Comment thread src/repository/opamGit.ml Outdated
Comment thread src/repository/opamGit.ml
Comment thread src/repository/opamGit.ml
Comment thread src/repository/opamGit.ml Outdated
Comment thread src/repository/opamGit.ml Outdated

@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.

lgtm! some tiny comments remaining, but the PR is good to merge

Comment thread src/client/opamCommands.ml Outdated
Comment thread src/repository/opamGit.mli Outdated
Note however that it cannot be used if you need values from the global git
configs (e.g. /etc/gitconfig or ~/.gitconfig) *)
val env : unit -> string array
val env : string array Lazy.t

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.

Suggested change
val env : string array Lazy.t
val env : unit -> string array

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If you want but i'd argue that it is dangerous to not make it explicit that the return value is memoised when returning a mutable value. Although this is a different issue and should be applied to OpamProcess.default_env too in a separate PR later. I've opened #7088 for that.

Comment thread src/repository/opamGit.ml Outdated
Comment thread src/repository/opamGit.ml
@kit-ty-kate
kit-ty-kate merged commit 5aa6df3 into ocaml:master Aug 20, 2026
41 checks passed
@kit-ty-kate
kit-ty-kate deleted the no-git-maintenance branch August 20, 2026 14:06
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.

2 participants