Fix bug where dev repo urls ending with a "/" would result in opam monorepo pull placing package source code directly inside the duniverse directory instead of in a subdirectory of the duniverse directory
#359
Conversation
gridbugs
force-pushed
the
fix-trailing-slash-in-dev-repo-path-bug
branch
from
1fe18f0
to
ab08f0a
Compare
There was a problem hiding this comment.
Good catch! It makes a lot of sense to make sure opam-monorepo pull doesn't write all over the users disk. I like that the tests also check for directory traversal out of the duniverse directory.
I've added some comments where I think things could be clearer, but I think the approach is good.
| Fpath.(is_prefix (normalize duniverse_dir) (normalize output_dir)) | ||
| && not (String.equal dir "") | ||
| then | ||
| let url = Url.to_opam_url url in |
There was a problem hiding this comment.
Maybe this part should be split into it's own function, since long if-blocks followed by rather large else blocks are hard to read.
There was a problem hiding this comment.
I split out the logic for deleting version control info and vendor dirs into its own function
gridbugs
force-pushed
the
fix-trailing-slash-in-dev-repo-path-bug
branch
from
ab08f0a
to
0e0c590
Compare
|
I've updated the code based on feedback but I haven't written any more tests yet. Will do tomorrow. |
gridbugs
force-pushed
the
fix-trailing-slash-in-dev-repo-path-bug
branch
from
0e0c590
to
d217190
Compare
|
Added tests for cases where dev_repos fail to parse into names. I also hit a bug where |
Previously these would be given a dev repo name of "" in the lockfile. This would result in `opam monorepo pull` checking out package source code directly inside the "duniverse" directory instead of in a subdirectory. Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
Previously, certain adversarial dev repo names (e.g. "", "..") would cause `opam monorepo pull` to create and delete files outside the duniverse directory, or to place package source code directly inside the duniverse directory instead of a subdirectory. This change adds a check which prevents code being pulled into directories which are not descendants of the duniverse directory. Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
gridbugs
force-pushed
the
fix-trailing-slash-in-dev-repo-path-bug
branch
from
40547af
to
64b718f
Compare
|
LGTM! :) |
CHANGES: ### Changed - Treat packages with no build commands as if they can be built with dune (tarides/opam-monorepo#355, @gridbugs) ### Fixed - Fix resolving refs of locally pinned repositories (tarides/opam-monorepo#326, tarides/opam-monorepo#332, @hannesm, @Leonidas-from-XIV) - Read the `compiler` flag from OPAM metadata thus classifying more packages correctly as base packages (tarides/opam-monorepo#328, @Leonidas-from-XIV) - Fix bug where dev repo urls ending with a "/" would result in `opam monorepo pull` placing package source code directly inside the duniverse directory instead of in a subdirectory of the duniverse directory (tarides/opam-monorepo#359, @gridbugs)
No description provided.