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

marcodelapierre · 2024-06-25T15:03:19Z

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.

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 Claudio De La Pierre <marco.delapierre@gmail.com>

…nges 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

netlify · 2024-06-25T15:03:34Z

✅ Deploy Preview for nextflow-docs-staging ready!

Name	Link
🔨 Latest commit	`69416fc`
🔍 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 · 2024-06-25T15:05:08Z

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

marcodelapierre · 2024-06-26T09:22:13Z

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 · 2024-06-26T12:33:23Z

[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

marcodelapierre and others added 30 commits January 15, 2024 17:29

Multiple revisions 1st: AssetManager, Cmd Clone Pull Run

7d027ec

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

removed revision paramter from clone pull run

106e8df

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

updated unit tests for AssetManager

0a9e6f8

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

Merge branch 'master' into add/mult_revisions

37d332e

AssetManager provider.revision assigned at AssetManager object creation

5a9b159

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

Codespell typo in changelog

94d34f3

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManager: removed checkout method

4744774

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

Merge branch 'master' into add/mult_revisions

d9e7b2b

assetmanager: fixed build signature, and unit tests

08718ef

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

assetmanager: one more build signature fix

242a600

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManager: fix for multi revs in find() method

35ad1e9

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

Multiple revisions: added for Cmds Drop, View, Config, Info

463b6f1

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

minor fixes to log outputs in CmdRun

2d3273e

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManager: documented new localPath schema

d2e5ffe

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

K8sDriverLauncher : added revision support

60c3c5e

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

updates to AssetManagerTest

4bf1e37

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

edit to AssetManagerTest, git.pull for TAGS does not output result of…

7e32111

… operation Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

minor fix in GitlabRepositoryProvider: using DEFAULT_BRANCH instead o…

3b2d461

…f "master" Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

Minor edits

2e6461e

Signed-off-by: Ben Sherman <bentshermann@gmail.com>

Merge branch 'master' into add/mult_revisions

a8d2dee

[ci fast] Merge branch 'master' into add/mult_revisions

eaf45d8

Merge branch 'master' into add/mult_revisions

ed17284

parametrised revision delimiter

b3f141d

Signed-off-by: Dr Marco De La Pierre <marco.delapierre@gmail.com>

nf pull: option to list or not revs for each project

6706bd9

Signed-off-by: Dr Marco De La Pierre <marco.delapierre@gmail.com>

nicer output for nf list -r

e62f543

Signed-off-by: Dr Marco De La Pierre <marco.delapierre@gmail.com>

minor edit to CmdList

bd19877

Signed-off-by: Dr Marco De La Pierre <marco.delapierre@gmail.com>

AssetManager: adding listRevisions method (work in progress)

c0268af

Signed-off-by: Dr Marco De La Pierre <marco.delapierre@gmail.com>

small tune to CmdList

aae2afc

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

minor tweak to CmdList

268ec79

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManager: listRevisions method now working

ac8d755

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

marcodelapierre added 22 commits June 20, 2024 16:29

remove use of getPulledRevisions

f27b115

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

fix for CmdDrop: use of DEFAULT_REVISION_DIRNAME

3d45ef3

CmdList: better output for default revision

06a2229

AssetManager, CmdDrop : some method signature updates

a77bd59

Merge branch 'master' into add/mult_revisions_jun_revision_map

28fe569

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManager.download(): using bareRepo for pulling, plus related cha…

3b9978b

…nges Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

CmdList and CmdPull: no need for manager.close()

20e6edc

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManagerTest: updated list tests

2199974

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManagerTest: fixed manifest test 1

36dab4d

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManagerTest: fixed all executed tests

2375bda

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

CmdPullTest fixed

ae835e8

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManagerTest: fixed tests that need github token

6769307

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManagerTest: add test for checkBareRepo

2cf9942

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManagerTest: added various tests for revToCommit methods

0c140de

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManagerTest: updated listRevs tests

923e200

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManager: update to revisionToCommitWithBareRepo

bac8797

Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

AssetManagerTest: removed unneeded variable

5de01cc

UpdateModuleTest: small makeup

b061203

UpdateModuleTest: fixed all unit tests

5a81758

UpdateModuleTest: minor edit

3052994

AssetManager: set revision and localPath outside constructor as separ…

6d6d7ae

…ate method ; plus related updates across codebase

CmdPull: allow -a as equivalent to -all

69416fc

marcodelapierre requested a review from a team as a code owner June 25, 2024 15:03

marcodelapierre requested a review from pditommaso June 25, 2024 15:03

marcodelapierre self-assigned this Jun 25, 2024

marcodelapierre mentioned this pull request Jun 25, 2024

Enable multiple revisions for project repos (first attempt 2024) #4659

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

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

Are you sure you want to change the base?

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

Conversation

✅ Deploy Preview for nextflow-docs-staging ready!