Pinning a `git` repo should record the hash of the repository in the lock

[Leonidas-from-XIV]

Desired Behavior

When we pin a git repo:

(lang dune 3.21)
(pin
  (url "git+file://$PWD/_repo")
  (package (name foo)))
(package
  (name main)
  (depends foo))

then at the moment locking will lock the repo URL. But it will not record the revision:

(version dev)
 
(build (all_platforms ((dune))))

(source
  (fetch
    (url git+file:///$PWD/_repo)))

(dev)

This means that lock files containing a git repo will

1. Not be reproducible
2. Require pulling the pinned git repo every time

What we want is to is that the revision is fixed at lock-time thus build will always use the same locked revision.

Example

This is how the pin of a git-repo should be recorded in a lock file:

(version dev)
 
(build (all_platforms ((dune))))

(source
  (fetch
    (url git+file:///$PWD/_repo#0123456789abcdef)))

(dev)

[Leonidas-from-XIV]

After discussing this with @shonfeder we encountered two questions to be answered:

1. What should locking do? Is a lock just a solution for the solver, so we can build the package? In this case of course locking exactly what the user inputs is sensible as we don't plan it to be reproducible. If we however see it as a portable solution that is reproducible then we should try to make sure that the user will get the same result every time. This could also mean disallowing local path pins or at least introducing warnings that the lock file solution will not be reproducible.
2. If we do decide that we want lockdir solutions to try to be reproducible what should the semantics be.

If we do decide that the lock dir should rewrite pins to be more reproducible there's multiple approaches we could take

Look up revision unless otherwise specified

• Pinning git+file://$PWD/_repo records git+file:///$PWD/_repo#0123456789abcdef to make sure the user gets the same solution in most cases.
• Pinning git+file:///$PWD/_repo#0123456789abcdef records the exact value as we know that the user has pinned this exact revision
• Pinning git+file:///$PWD/_repo#branch records git+file:///$PWD/_repo#branch as the user has stated that they want to follow the branch branch. This is essentially an opt-out of reproducibility and will require Dune to pull the newest state of the repo every time.

Look up revision always

• Pinning git+file://$PWD/_repo records git+file:///$PWD/_repo#0123456789abcdef to make sure the user gets the same solution in most cases.
• Pinning git+file:///$PWD/_repo#0123456789abcdef records the exact value as we know that the user has pinned this exact revision
• Pinning git+file:///$PWD/_repo#branch records git+file:///$PWD/_repo#abcdef0123456789 as that was the state of the branch when we locked.

There is no way to opt-out of it, if the user wants to use a new version of the pin, they have to create a new lock dir. Or pin a file path.

[Alizter]

I think the second one makes the most sense here. The first one is non-nonsensical because we are allowing lock directories to have their meaning modified from the outside. This will make it impossible to share etc. If users wish to not lock, they are free to fallback to autolocking. We should test that this at least works for such a use case, but its not the biggest priority right now.

[rgrinberg]

This could also mean disallowing local path pins or at least introducing warnings that the lock file solution will not be reproducible.

I think that would just be annoying for local development. Local paths do not really break reproducibility anyway. The promise is that if sources resolve, they will resolve to the same thing that whoever produced the lock directory resolved them to. For local paths, the sources will not resolve in the first place.

I don't think the first option is nonsensical, and I can see how it can be useful in some cases. I agree that the second option is the more useful one in general. Don't be surprised if some users ask for 1 in some situations though.

[Leonidas-from-XIV]

Don't be surprised if some users ask for 1 in some situations though.

Yes, so I wonder how to signal that a branch should be tracked instead of resolving it. We could add a stanza to distinguish but it is more configuration. Maybe it would make sense to implement the second option as a default and then see the interest for the first option?

[Alizter]

@Leonidas-from-XIV You could emit a warning when locking that the url foo#branch was resolved to foo#afeafeaf.

[Leonidas-from-XIV]

But the warning will only make the user aware that the branch has been resolved (and the warning would always be printed unless the user pins a commit directly).

However if the user wants the branch to be tracked we'd need to add some configuration option, but adding configuration is messy and confusing (as you have to explain not one set of semantics but multiple) so if we can agree on one sensible default I think it would make for a better user experience.

[rgrinberg]

Yes, so I wonder how to signal that a branch should be tracked instead of resolving it. We could add a stanza to distinguish but it is more configuration. Maybe it would make sense to implement the second option as a default and then see the interest for the first option?

Let's wait until there's demand for this from users first.

You could emit a warning when locking that the url foo#branch was resolved to foo#afeafeaf.

This warning would also likely be noise. Commit hashes don't really mean anything to users.

[Leonidas-from-XIV]

To summarize our conclusions from yesterdays dev-meeting: We should always be resolving the git pins to the revisions that we used when locking. If users want their pinned repo to be updated every time they can use file:// instead of git.

However we should implement a warning when pinning a local git repo which has uncommitted local changes ("dirty").