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

opam update fails because of custom git settings diff.noprefix #3627

Closed
Blaisorblade opened this issue Oct 24, 2018 · 1 comment · Fixed by #3788
Closed

opam update fails because of custom git settings diff.noprefix #3627

Blaisorblade opened this issue Oct 24, 2018 · 1 comment · Fixed by #3788
Milestone

Comments

@Blaisorblade
Copy link
Contributor

Blaisorblade commented Oct 24, 2018

opam update fails on the iris-dev repo if my ~/.gitconfig contains:

[diff]
        noprefix = true

That changes the output of git diff to default to -p0 not -p1 — but opam generates a diff with git "diff" "--no-ext-diff" "-R" "-p" "refs/remotes/opam-ref" "--" and tries to apply that with patch -p1 during opam update.

The minimal possible fix seems to reset this option via git -c diff.noprefix=false "diff" (I might try this), but always skipping the local git config files could be more robust.

man git has this suggestion:

       GIT_CONFIG_NOSYSTEM
           Whether to skip reading settings from the system-wide $(prefix)/etc/gitconfig file. This environment variable can be used along with $HOME and $XDG_CONFIG_HOME to create a predictable environment
           for a picky script, or you can set it temporarily to avoid using a buggy /etc/gitconfig file while waiting for someone with sufficient permissions to fix it.

Command outputs

Install works:

$ opam repo add --all-switches iris-dev https://gitlab.mpi-sws.org/FP/opam-dev.git
[iris-dev] Initialised

<><> Upgrading repositories from older opam format ><><><><><><><><><><><><>  🐫
Upgrading repository "iris-dev"...

Update doesn't:

$ opam update

<><> Updating package repositories ><><><><><><><><><><><><><><><><><><><><>  🐫
[coq-released] synchronised from http://coq.inria.fr/opam/released
[iris-dev] synchronised from git+https://gitlab.mpi-sws.org/FP/opam-dev.git
[ERROR] Could not update repository "iris-dev": "/usr/bin/patch -p1 -i /Users/pgiarrusso/.opam/log/processed-patch-22224-c743ac" exited with code 1
[default] no changes from https://opam.ocaml.org/

<><> Upgrading repositories from older opam format ><><><><><><><><><><><><>  🐫
Upgrading repository "coq-released"...

<><> Synchronising development packages <><><><><><><><><><><><><><><><><><>  🐫
[coq-iris-builddep.dev] no changes from file:///Users/pgiarrusso/git/Coq/iris-coq/build-dep
[coq-stdpp.dev.2018-10-15.1.b7f1f6b1] no changes from git+https://gitlab.mpi-sws.org/robbertkrebbers/coq-stdpp.git#b7f1f6b178091ca6e3d363208d7875d73321903c

After undoing the migration via cd ~/.opam/repo/iris-dev; git reset HEAD; git checkout .; git clean -fdx, the update works again, once.

The failure appears to be when running patch -p1 against a -p0 patch generated by /usr/local/bin/git "diff" "--no-ext-diff" "-R" "-p" "refs/remotes/opam-ref" "--".

Full output of opam update -vv (warning: 6M) here:
https://gist.github.com/Blaisorblade/24189e423677a61e524273d705091c6c. The issue is that git diff is indeed supposed to generate -p1 patches.

Blaisorblade added a commit to Blaisorblade/opam that referenced this issue Oct 24, 2018
Try fixing ocaml#3627 as suggested there. To test.
Blaisorblade added a commit to Blaisorblade/opam that referenced this issue Oct 24, 2018
Fix ocaml#3627 by ensuring git produces a `-p1` diff. This is a very limited
fix but appears to work in my usecase. I have reviewed the other uses of
`git diff` in the same file, but it appears their output is not passed
to `patch` hence this change is unnecessary.
@rjbou
Copy link
Collaborator

rjbou commented Oct 25, 2018

Thanks for the detailed report. Sometimes, we have to overwrite some user git configuration to have git behaves like opam needs to. We can add in the opam internal git repo local config option to disable diff.noprefix (cf. pr).

always skipping the local git config files could be more robust.

There is no perfect solution about config, both have their wrong side-effect: keeping config interferes with opam/git expectation, but discarding it can also leads to some misbehaviour e.g. if git is configured to use a proxy.

@rjbou rjbou added this to the 2.1.0 milestone Feb 28, 2019
rjbou pushed a commit to rjbou/opam that referenced this issue Mar 20, 2019
Fix ocaml#3627 by ensuring git produces a `-p1` diff. This is a very limited
fix but appears to work in my usecase. I have reviewed the other uses of
`git diff` in the same file, but it appears their output is not passed
to `patch` hence this change is unnecessary.
rjbou added a commit that referenced this issue Mar 20, 2019
Fix #3627 by ensuring git produces a `-p1` diff. This is a very limited
fix but appears to work in my usecase. I have reviewed the other uses of
`git diff` in the same file, but it appears their output is not passed
to `patch` hence this change is unnecessary.
rjbou added a commit to rjbou/opam that referenced this issue Mar 27, 2019
Fix ocaml#3627 by ensuring git produces a `-p1` diff. This is a very limited
fix but appears to work in my usecase. I have reviewed the other uses of
`git diff` in the same file, but it appears their output is not passed
to `patch` hence this change is unnecessary.
rjbou added a commit to rjbou/opam that referenced this issue Mar 27, 2019
Fix ocaml#3627 by ensuring git produces a `-p1` diff. This is a very limited
fix but appears to work in my usecase. I have reviewed the other uses of
`git diff` in the same file, but it appears their output is not passed
to `patch` hence this change is unnecessary.
rjbou added a commit to rjbou/opam that referenced this issue Mar 28, 2019
Fix ocaml#3627 by ensuring git produces a `-p1` diff. This is a very limited
fix but appears to work in my usecase. I have reviewed the other uses of
`git diff` in the same file, but it appears their output is not passed
to `patch` hence this change is unnecessary.
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 a pull request may close this issue.

2 participants