-
Notifications
You must be signed in to change notification settings - Fork 739
Enable multiple git revisions (Jun 2024) : final, using commit ID in repo localPath #5089
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
Conversation
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
... operation Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
...f "master" Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Dr Marco De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>
...ate method ; plus related updates across codebase
✅ Deploy Preview for nextflow-docs-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Note: some commits were not signed-off (sigh) - leaving as is for now
Extra note on the method AssetManager.download() - corner case.
It is worthwhile doing some tests on what happens in the following corner case:
- both a revision and branch/commit are pulled, which are pointing to the same commit,
- then the branch gets new commits in the remote,
- then the pulls are updated.
It is worth testing against distinct pull orders:
- tag/commit -> branch -> [update remote branch] -> tag/commit
- branch -> tag/commit -> [update remote branch] -> tag/commit
If this code block works as expected, it should be all fine.
I will see if I can make the test soon, and in case will update this thread, otherwise I will leave it for later.
[UPDATE] done some testing, the code block mentioned above indeed does its job, hence this corner case is OK.
PS: one learning I made here is that, when a tag is requested, .resolve + .getName in JGit return a sort of aliased commit, i.e. a commit that does not exist in the repo history, but that points to the correct real commit that corresponds to the requested tag.
[UPDATE] this issue exists already with Nextflow stable release, it was not introduced by this PR.
Issue with a test repo which has lots of tags:
On first pull all good, however see how the commit is made to refer via distance from a tag, tag4~17:
$ NXF_HOME=$(pwd)/HOME_PR ./launch.sh pull -r 451ebd9dcb56045d80963945305811aa65f413d0 marcodelapierre/hello-nf
Checking marcodelapierre/hello-nf:451ebd9dcb56045d80963945305811aa65f413d0 ...
WARN: Cannot read project manifest -- Cause: Remote resource not found: https://api.github.com/repos/marcodelapierre/hello-nf/contents/nextflow.config?ref=451ebd9dcb56045d80963945305811aa65f413d0
downloaded from https://github.com/marcodelapierre/hello-nf.git - revision: 451ebd9dcb [tag4~17]
On repeated pull:
$ NXF_HOME=$(pwd)/HOME_PR ./launch.sh pull -r 451ebd9dcb56045d80963945305811aa65f413d0 marcodelapierre/hello-nf
Checking marcodelapierre/hello-nf:451ebd9dcb56045d80963945305811aa65f413d0 ...
Remote origin did not advertise Ref for branch refs/tags/tag4~17. This Ref may not exist in the remote or may be hidden by permission settings.
In good contrast, for a repo with not as many tags, all good:
$ NXF_HOME=$(pwd)/HOME_PR ./launch.sh pull -r 3650d7b8371c255a0a89ed5f2e07e2e5c1ed29ae hello
Checking nextflow-io/hello:3650d7b8371c255a0a89ed5f2e07e2e5c1ed29ae ...
downloaded from https://github.com/nextflow-io/hello.git - revision: 3650d7b8371c255a0a89ed5f2e07e2e5c1ed29ae
$ NXF_HOME=$(pwd)/HOME_PR ./launch.sh pull -r 3650d7b8371c255a0a89ed5f2e07e2e5c1ed29ae hello
Checking nextflow-io/hello:3650d7b8371c255a0a89ed5f2e07e2e5c1ed29ae ...
Already-up-to-date - revision: 3650d7b8371c255a0a89ed5f2e07e2e5c1ed29ae
1f834a0 to
6454605
Compare
5a93547 to
27345a6
Compare
f6a3696 to
49b58d2
Compare
b4b321e to
069653d
Compare
b7b4221 to
c1114bc
Compare
Uh oh!
There was an error while loading. Please reload this page.
This PR is spawned out of #4659, of which it represents its evolution and finalisation.
Some highlights:
localPathdefined via thecommitId, for more accurate handling of branch updatesrevisionMapfile, for accurate handling of user requested revisions vs revision update in remote repoAssetManagerclass constructor; a dedicated additional method,setRevisionAndLocalPath, has to be called after instantiation to setlocalPathandrevisionSome implementation notes:
projectNamedetermines location ofBARE_REPO,REVISION_MAPandREVISION_SUBDIRlocalPathis underREVISION_SUBDIRand requires revision-to-commit mapping ; for unit testing, redefine thelocalPathmanuallySome caveats:
revisionMapfile: I/O concurrency, refactor to a dedicated class (similar toScriptFile), others?listoutput:commitId-basedlocalPathAssetManager, some non-constructor methods have changed API, typically to get rid of the now un-neededrevisionargumentdownload()andclone()@pditommaso for visibility.