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

Git --depth 1 breaks orb/dune #6145

Closed
reynir opened this issue Aug 8, 2024 · 5 comments · Fixed by #6146
Closed

Git --depth 1 breaks orb/dune #6145

reynir opened this issue Aug 8, 2024 · 5 comments · Fixed by #6146

Comments

@reynir
Copy link
Contributor

reynir commented Aug 8, 2024

First of all congratulations on releasing opam 2.2! 🥳

In orb we recently updated to the version 2.2 opam libraries in robur-coop/orb#15 and robur-coop/orb#17. It builds successfully, but when we use the new orb to build itself again we observe an error on Debian: https://builds.robur.coop/job/orb/build/955defa7-9ff5-447b-b67b-4e928d80cda4

The error is that dune subst produces a version number da200fd instead of the expected 0.0.2-25-gda200fd.

After some investigation with @hannesm we believe it may be due to #4442 where a shallow git clone is done now. Dune uses git tags to derive the version number, and with a shallow clone these are no longer available.

From our point of view we didn't find a way to force a deep clone.

@hannesm
Copy link
Member

hannesm commented Aug 8, 2024

To add a bit more, we use the following API in orb:

Due to the intricate build of a MirageOS unikernel, we use pull_tree -- but in the common case we use OpamClient.install... and rely on dune subst to emit a version number (0.0.2-yy-zz) -- which with opam 2.2 is now (zz), missing the git tag information.

@kit-ty-kate
Copy link
Member

I had a go at adding the missing ?full_fetch however i think this is too big of a change for such a feature we don't use and which would be easy to break in the future.

What about we instead introduce a new full_fetch option to OpamRepositoryConfig and the associated OPAMFULLFETCH environment variable? This way would make the code change much simpler and would also allow users of the opam binary to force this behaviour too if they need to.

@kit-ty-kate
Copy link
Member

Thinking about this further i think #4442 use-case was for the opam-repository fetch, for regular packages maybe it would make sense to keep full_fetch:true by default as they might depend on some git information

@kit-ty-kate
Copy link
Member

I've opened #6146 which implements my last comment

@reynir
Copy link
Contributor Author

reynir commented Aug 8, 2024

I think it makes sense to default to full fetch for packages. I tried the following on opam 2.2 and opam 2.1.4:

$ opam pin -y orb https://github.com/robur-coop/orb.git#da200fdb9affdb683bf4db56078a5111253a8ed8
$ orb --version
  • on 2.2 I get: da200fd
  • on 2.1.4 I get: v0.0.2-25-gda200fd

So this change also affects the opam command line utility.

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

Successfully merging a pull request may close this issue.

4 participants