ocaml / opam

opam is a source-based package manager. It supports multiple simultaneous compiler installations, flexible package constraints, and a Git-friendly development workflow.
https://opam.ocaml.org
Other
1.25k stars 361 forks source link

opam update patch fails on a file->dir rename in opam repository #3830

Open avsm opened 5 years ago

avsm commented 5 years ago

the seemingly innocuous ocaml/opam-repository@3044264138073fb14576d59b8278c4f8f9b43e51 change broke all opam update calls in CI immediately with:

[default] synchronised from git+file:///home/opam/opam-repository
[ERROR] Could not update repository "default": "/usr/bin/patch -p1 -i /home/opam/.opam/log/processed-patch-6-58c514" exited with code 1
The command '/bin/sh -c opam update' returned a non-zero code: 40
The command "bash -ex ./.travis-docker.sh" exited with 40.

Looks like its due to a file -> directory rename which is tripping up patch; see https://github.com/ocaml/opam-repository/pull/13992/files for the PR that triggered it.

avsm commented 5 years ago

triage from @rjbou reveals patch aint patching:

$ cat ~/.opam/log/processed-patch-22283-2dea01
diff --git b/packages/omd/omd.2.0.0~alpha a/packages/omd/omd.2.0.0~alpha/opam
similarity index 100%
rename from packages/omd/omd.2.0.0~alpha
rename to packages/omd/omd.2.0.0~alpha/opam
$ patch -p1 "-i" "~/.opam/log/processed-patch-22283-2dea01"
Invalid file name packages/omd/omd.2.0.0~alpha/opam -- skipping patch
rjbou commented 5 years ago

Confirmed, patch don't handle renaming a file into a directory. One more reason to rely on another patch implementation.

$ find t_patch
t_patch
t_patch/foo

$ cat /tmp/p
diff --git a/t_patch/foo b/t_patch/foo/opam
similarity index 100%
rename from t_patch/foo
rename to t_patch/foo/opam

$ patch -p1 -i /tmp/p
Invalid file name t_patch/foo/opam -- skipping patch
dra27 commented 5 years ago

There's also the --no-renames for git diff which may help reduce the number of these things which appear (I think that the diff would then be a deletion of the old file followed by a new file)

hannesm commented 5 years ago

or, as opam does invoke git diff, require a combined diff (the -c argument to git diff), which uses less extension headers than the common git diff (thus it is likely patch will be fine with it)...