Enable multiple git revisions (Jun 2024) : final, using commit ID in repo localPath

marcodelapierre commented 3 months ago

This PR is spawned out of #4659, of which it represents its evolution and finalisation.

Some highlights:

handles multiple revisions by storing them in localPath defined via the commitId, for more accurate handling of branch updates
uses a local bare repository under the hood to both get revision->commit mapping and to pull required revisions
stores revision->commit mapping into a dedicated revisionMap file, for accurate handling of user requested revisions vs revision update in remote repo
maintains original API for AssetManager class constructor; a dedicated additional method, setRevisionAndLocalPath, has to be called after instantiation to set localPath and revision
docs and unit tests updated

Some implementation notes:

when dropping a revision, all revisions pointing to the same commit are also dropped
repo directory layout:
- projectName determines location of BARE_REPO, REVISION_MAP and REVISION_SUBDIR
- localPath is under REVISION_SUBDIR and requires revision-to-commit mapping ; for unit testing, redefine the localPath manually

Some caveats:

to be improved: handling of revisionMap file: I/O concurrency, refactor to a dedicated class (similar to ScriptFile), others?
for now, default branch appear duplicated in revisionMap and hence in the list output:
- files are never duplicated thanks to commitId-based localPath
- however running with no revision or with default revision may point to different commit IDs, if only one is updated to latest
in AssetManager, some non-constructor methods have changed API, typically to get rid of the now un-needed revision argument
- examples include download() and clone()
- it should be easy to restore these if needed
(minor) tedious Warning messaging (missing manifest) when pulling tags or non-existing revisions
- this behaviour is pre-existing to this PR (only when first pull of a project is done requesting a tag)
- it is seen more often, because every pull of a new revision is a fresh pull

@pditommaso for visibility.

netlify[bot] commented 3 months ago

Deploy Preview for nextflow-docs-staging ready!

Name	Link
Latest commit	69416fc8a0c5158d83d9dc4b190adcac92c8a91c
Latest deploy log	https://app.netlify.com/sites/nextflow-docs-staging/deploys/667adc3af1e92900086784d6
Deploy Preview	https://deploy-preview-5089--nextflow-docs-staging.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

marcodelapierre commented 3 months ago

Note: some commits were not signed-off (sigh) - leaving as is for now

marcodelapierre commented 3 months ago

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.

marcodelapierre commented 3 months ago

[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

nextflow-io / nextflow