Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Recursive & subpath based pin #3499

Merged
merged 35 commits into from
Mar 11, 2020
Merged

Recursive & subpath based pin #3499

merged 35 commits into from
Mar 11, 2020

Conversation

rjbou
Copy link
Collaborator

@rjbou rjbou commented Aug 8, 2018

This PR is the continuation of #3174, started by @hnrgrgr.

It introduces recursive, subpath based, and subpath recursive pin, with options --recursive and --subpath PATH.

On recursive mode, opam goes throught subdirectories to find opam files & pin them to their subdirectories.
On subpath based mode, opam goes to the given subdirectory to find opam file & pin it to this subdrectory.

Mixed pins are not allowed, pin kind is the top directory kind. If git repos are subdirectory of a local directory, thoses packages will be pinned as path based. If a subdirectory is not versionned of a top git directory, it is ignored (as it is git-based copy).

Internally, opam doesn't copy the whole top directories (except for hg & darcs), only the content of subdirectory of each subpath based package and its full directory structure from the top directory. Also, for git+http backend, git repository is downloaded only once, sub pins are retrieved from pin cache.

These options are available for: pin, unpin, remove, install, reinstall, upgrade, and lint. opam source also handles it, it copies the sources of the subdirectory content only.

A pin example can be found in this gist.

Note: This PR is a WIP, it needs to be tested, and at the end git tree need to be cleaned before merge.
Closes #3174
Closes #3477

@rjbou rjbou requested a review from AltGr August 8, 2018 17:51
@rjbou rjbou force-pushed the recursive-pin branch 2 times, most recently from 868445d to 8910d30 Compare August 10, 2018 09:57
@rjbou
Copy link
Collaborator Author

rjbou commented Aug 10, 2018

@jberdine

I worry about having to sync the whole repo

On this implementation, the whole repo in synced only once, at first time, then from this local pull, only package subdirectory is copied internally. After, for update check, only subdirectories are checked/retrieved (local & git backends).

I am a bit surprised at how tightly integrated opam and git are though (e.g. also what happens if I want to use hg? :-) ).

opam handles hg and darcs VCS also. For the moment, in this PR, you can use rec/sub pin, but not have all its optimisations (full repo is cloned & checked up).
I'm not familiar with hg and darcs, any input is welcome ;)

@jberdine
Copy link

I tried testing this a bit.
I created a pin with

opam pin add --no-action --working-dir --inplace-build --kind=git --subpath my_package_subdir my_package ..

where .. is the git repo root, ./_opam is the dir containing the current local switch, and ../my_package_subdir is the current dir, and ../my_package_subdir/my_package.opam is the opam file for my_package.
Should I expect a copy of pretty much the full git repo's .git dir to be placed in _opam/.opam-switch/sources? I was really hoping to avoid that. Just rsyncing the files listed by git ls-files was really slick.

Also, when then trying to install the pinned package, the path the build starts from seems not to use the subpath, causing build failure. Is that expected? For example, in:

#=== ERROR while compiling my_package.~dev =========================================#
# context     2.0.0 | macos/x86_64 |  | pinned(git+file:///Users/jjb/root#wip#18285334)
# path        ~/root
# command     ~/.opam/opam-init/hooks/sandbox.sh build make dunes
# exit-code   2
# env-file    ~/.opam/log/my_package-7844-5f1023.env
# output-file ~/.opam/log/my_package-7844-5f1023.out

I expected # path ~/root/my_project_subdir instead of # path ~/root.

Note that there are warnings such as

[ERROR] Cannot sync /Users/jjb/root/ into /Users/jjb/root/my_package_subdir/_opam/.opam-switch/sources/my_package: they
        overlap

at various times. It seems that having a local switch in a subdir of the git repo is also trouble.

I also do not understand --assume-built, as with it no files seem to be installed into ../_opam/lib, etc. at all. I interpreted the 'built' assumption as meaning built in the working tree, but not installed into the _opam switch's tree. Perhaps this is a casualty of the other steps not quite working.

@AltGr
Copy link
Member

AltGr commented Aug 13, 2018

--working-dir --inplace-build

You're not testing the simplest case here... Which is good! Thanks ;)
Note however that inplace-build supersedes working-dir, since the latter says to copy the latest version of files into the build dir, but the former tells to not use a separate build dir at all and work from the pinning source directly.

Should I expect a copy of pretty much the full git repo's .git dir to be placed in _opam/.opam-switch/sources?

Git doesn't offer subtree cloning like e.g. SVN does. So we need to get the full .git/, but should then only checkout the required subtree (@rjbou correct me if I am wrong!)
But since you used --working-dir, rsync is used instead anyways.

Also, when then trying to install the pinned package, the path the build starts from seems not to use the subpath, causing build failure. Is that expected?

Probably a bug related to --inplace-build, we'll check that, thanks!

Note that there are warnings such as

[ERROR] Cannot sync /Users/jjb/root/ into /Users/jjb/root/my_package_subdir/_opam/.opam-switch/sources/my_package: they overlap

that's weird, we normally explicitely exclude _opam/ from calls to rsync for that precise reason. Will check.

@jberdine
Copy link

jberdine commented Aug 13, 2018 via email

@jberdine
Copy link

jberdine commented Aug 13, 2018 via email

@rjbou
Copy link
Collaborator Author

rjbou commented Aug 13, 2018

First, thanks for testing this PR. Plus, as you have a bunch of corner cases, it is a good completnes & "stress" test :).

Now the answers...

Should I expect a copy of pretty much the full git repo's .git dir to be placed in _opam/.opam-switch/sources?

Git doesn't offer subtree cloning like e.g. SVN does. So we need to get the full .git/, but should then only checkout the required subtree (@rjbou correct me if I am wrong!)

Indeed, the checkout is done from the git root, but only subdirectories are checkout (with sparse checkout enabled), e.g. if you have in your repo

gitroot/.git:
[...]
gitroot/foo:
f1 f2
gitroot/bar:
f3 f4

and you pin bar with a subpath, you'll have in the internal opam folder

gitroot/.git:
[...]
gitroot/bar:
f3 f4

Have you considered whether using git worktree would be feasible? For large repos that could save a lot of space and IO.

I just had a quick look to git worktree, it can be useful for git local pinning more generally. One of my concerns would be to depends on a new version of git (it was introduced in git 2.5). I'll look more in it!

Also, when then trying to install the pinned package, the path the build starts from seems not to use the subpath, causing build failure. Is that expected?
Probably a bug related to --inplace-build, we'll check that, thanks!

It's not from the inplace-build option, but the pinning path. As we can see in this example the pinning path is the one of the root directory, and a subpath is also defined. In this case, the subpath was not printed in the error log, while it should.

Note that there are warnings such as

    [ERROR] Cannot sync /Users/jjb/root/ into /Users/jjb/root/my_package_subdir/_opam/.opam-switch/sources/my_package: they overlap

that's weird, we normally explicitely exclude _opam/ from calls to rsync for that precise reason. Will check.

It's because of the subpath pinning. _opam directory is well excluded from the rsync, and it's not even performed. A prior check fails because of the pinning path that doesn't contain the subpath (cf. over). I'll add a fix.

My understanding is that with inplace-build the uncommitted changes will NOT be ignored, correct?

It's the working-dir, with git backend, that includes uncommitted changes (by changing the backend to rsync). The inplace-build performs the build locally instead of in the opam build internal directory.

I also do not understand --assume-built, as with it no files seem to be installed into ../_opam/lib, etc. at all. I interpreted the 'built' assumption as meaning built in the working tree, but not installed into the _opam switch's tree. Perhaps this is a casualty of the other steps not quite working.

It assumes that everything has been built in the working tree, and installs it in the opam switch tree, here the local _opam switch.

@jberdine
Copy link

jberdine commented Aug 13, 2018 via email

@rjbou
Copy link
Collaborator Author

rjbou commented Aug 27, 2018

@jberdine

Is --depth 1 or similar used to limit the history that is fetched/copied?

No, for the moment, full repo is fetched. @AltGr, is there a reason for not using it ?

Are you saying that I ought to have used different paths when setting up the pin?

No, you used correctly the command, subpath was missing in the error report. The url pin with subpath is not the full path: url pin remains the directory given as arguments, and the subpath is used internally to work with, so for display they are dissociated (cf example).

I updated the PR to adjust subpath behavior with working-dir: you should'nt have anymore those errors (error report / overlap / etc.).

@dra27
Copy link
Member

dra27 commented Oct 11, 2018

Just a note that Git worktree is annoyingly not present on CentOS/RHEL installations of Git, as it still ships Git 1.8.

@rjbou rjbou force-pushed the recursive-pin branch 8 times, most recently from 30b4ca7 to 1f61a21 Compare January 18, 2019 16:00
@rjbou
Copy link
Collaborator Author

rjbou commented Feb 27, 2019

Final update before merge!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

opam pin add -k git in 1.2.2 vs 2.0
5 participants