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

TODO: Have a look at the patches from Debian #5058

Open
kit-ty-kate opened this issue Feb 15, 2022 · 10 comments
Open

TODO: Have a look at the patches from Debian #5058

kit-ty-kate opened this issue Feb 15, 2022 · 10 comments

Comments

@kit-ty-kate
Copy link
Member

https://salsa.debian.org/ocaml-team/opam/-/tree/master/debian/patches

@AltGr
Copy link
Member

AltGr commented Feb 15, 2022

https://salsa.debian.org/ocaml-team/opam/-/blob/master/debian/patches/0004-Use-HOME-env-variable-instead-of.patch should really not change anything since the functions called (e.g. OpamFilename.of_string) detect the ~ and rewrite it using HOME in the exact same way the patch does 🤔

I am guessing the "fix" might come from a lintian check that frowns upons strings like ~/.cache in code.

@kit-ty-kate
Copy link
Member Author

cc @glondu

@glondu
Copy link

glondu commented Feb 16, 2022

@mehdid did the patch... I don't have more explanation than what's written in the patch :-/ I will try to remove it in the next upload to see what happens.

@mehdid
Copy link
Contributor

mehdid commented Feb 17, 2022

IIRC, it had to do with reproducible builds. Using ~ doesn't ensure the path is the same after each build. So I set HOME in https://salsa.debian.org/ocaml-team/opam/-/blob/master/debian/rules and use it in the code to ensure it remains always the same.

@kit-ty-kate
Copy link
Member Author

@mehdid but as @AltGr explained this is already exactly what OpamFilename.Dir.of_string does. What is then the difference?

@kit-ty-kate
Copy link
Member Author

I had a look at the patches again and noticed a new patch ported over from Fedora (https://src.fedoraproject.org/rpms/opam/c/b2a3f3e63c552c7fd7060da41c556f77a907d394?branch=rawhide) @jamesjer what is the reason for this patch?

@kit-ty-kate
Copy link
Member Author

btw, small question for the Debian maintainers: is there a reason why opam on Debian Stable is stuck at 2.1.2 ? I'm currently releasing 2.1.6 but 2.1.2 is more than 2 years old

@jamesjer
Copy link

When we introduced dune 3.11 into Fedora, opam started failing to build like this:

Running[1]: (cd _build/default/src/client && /usr/bin/sh -c 'git describe --exact HEAD || echo [dev]') > _build/default/src/client/git-describe 2> /dev/null
Running[2]: (cd _build/default/src/client && /usr/bin/sh -c 'git rev-parse --quiet --verify HEAD || echo .') > _build/default/src/client/git-sha 2> /dev/null
sed -f process.sed opam.install > processed-opam.install
sed: can't read opam.install: No such file or directory

The patch avoids the build failure. I don't know why this didn't happen with dune 3.10 and earlier.

@kit-ty-kate
Copy link
Member Author

I don't believe the patch does the right thing but in any case this failure with dune 3.11 was fixed in 2.1.6 so you should be able to remove this patch

@jamesjer
Copy link

Thank you. I will do so.

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

No branches or pull requests

5 participants