From d1ebdcc9de3b567ec8c46a593c1a789f190968d8 Mon Sep 17 00:00:00 2001 From: Edmund Miller Date: Wed, 18 Dec 2024 07:36:53 -0600 Subject: [PATCH] fix: Handle release candidate versions --- .../groovy/nextflow/config/Manifest.groovy | 113 +++++++++++++++++- .../nextflow/scm/AssetManagerTest.groovy | 75 ++++-------- 2 files changed, 129 insertions(+), 59 deletions(-) diff --git a/modules/nextflow/src/main/groovy/nextflow/config/Manifest.groovy b/modules/nextflow/src/main/groovy/nextflow/config/Manifest.groovy index 87024ea279..aad584a3e1 100644 --- a/modules/nextflow/src/main/groovy/nextflow/config/Manifest.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/config/Manifest.groovy @@ -55,12 +55,17 @@ class Manifest { String getDefaultBranch() { - target.defaultBranch + target.defaultBranch ?: 'master' } - // 1. if defaultRevision is set, use that - // 2. if version is set, use that - // 3. Fallback to defaultBranch + /** + * Gets the default revision to use + * Priority order: + * 1. defaultRevision if set + * 2. version if set + * 3. defaultBranch + * @return The revision to use + */ String getDefaultRevision() { target.defaultRevision ?: getVersion() ?: getDefaultBranch() } @@ -111,6 +116,10 @@ class Manifest { target.nextflowVersion } + /** + * Gets the version + * @return The version string or null if not set + */ String getVersion() { target.version } @@ -201,4 +210,100 @@ class Manifest { MAINTAINER, CONTRIBUTOR } + + /** + * Checks if the current version is a development version + * @return true if version ends with 'dev' or '-dev', false otherwise + */ + boolean isDevelopmentVersion() { + def version = getVersion() + if (!version) return false + return version.endsWith('dev') || version.endsWith('-dev') + } + + /** + * Checks if the current version is a release candidate + * @return true if version contains '-RC', false otherwise + */ + boolean isReleaseCandidate() { + def version = getVersion() + if (!version) return false + return version.contains('-RC') + } + + /** + * Checks if the current version is a hotfix + * @return true if version contains '-hotfix', false otherwise + */ + boolean isHotfix() { + def version = getVersion() + if (!version) return false + return version.contains('-hotfix') + } + + /** + * Validates if a version string is a valid semantic version + * @param version The version string to validate + * @return true if valid, false otherwise + */ + boolean isValidVersion(String version) { + if (!version) return false + // Matches standard versions like 1.0.0, release candidates like 1.0.0-RC1, development versions like 1.0.0dev, and hotfixes like 1.0.0-hotfix + version ==~ /^\d+\.\d+\.\d+(-RC\d+|-dev|dev|-hotfix)?$/ + } + + /** + * Compares two version strings + * @return -1 if v1 < v2, 0 if v1 == v2, 1 if v1 > v2 + */ + private int compareVersions(String v1, String v2) { + def v1Parts = v1.tokenize('.') + def v2Parts = v2.tokenize('.') + + for (int i = 0; i < Math.min(v1Parts.size(), v2Parts.size()); i++) { + def v1Part = v1Parts[i].replaceAll(/(-RC\d+|-dev|dev|-hotfix)$/, '') as int + def v2Part = v2Parts[i].replaceAll(/(-RC\d+|-dev|dev|-hotfix)$/, '') as int + if (v1Part != v2Part) { + return v1Part <=> v2Part + } + } + + v1Parts.size() <=> v2Parts.size() + } + + /** + * Checks if the current version is greater than the provided version + * @param otherVersion The version to compare against + * @return true if current version is greater, false otherwise + */ + boolean isVersionGreaterThan(String otherVersion) { + def version = getVersion() + if (!version || !otherVersion) return false + + // Strip any suffixes for comparison + def v1 = version.replaceAll(/(-RC\d+|-dev|dev|-hotfix)$/, '') + def v2 = otherVersion.replaceAll(/(-RC\d+|-dev|dev|-hotfix)$/, '') + + compareVersions(v1, v2) > 0 + } + + /** + * Checks if the current version is compatible with the provided version + * @param otherVersion The version to check compatibility with + * @return true if compatible, false otherwise + */ + boolean isVersionCompatibleWith(String otherVersion) { + def version = getVersion() + if (!version || !otherVersion) return false + + // Development versions are always compatible + if (isDevelopmentVersion()) return true + + // For release candidates and hotfixes, compare base versions + def v1 = version.replaceAll(/(-RC\d+|-dev|dev|-hotfix)$/, '') + def v2 = otherVersion.replaceAll(/(-RC\d+|-dev|dev|-hotfix)$/, '') + + // Version should be greater than or equal to the other version + compareVersions(v1, v2) >= 0 + } } diff --git a/modules/nextflow/src/test/groovy/nextflow/scm/AssetManagerTest.groovy b/modules/nextflow/src/test/groovy/nextflow/scm/AssetManagerTest.groovy index 24a393a49e..38aaf45186 100644 --- a/modules/nextflow/src/test/groovy/nextflow/scm/AssetManagerTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/scm/AssetManagerTest.groovy @@ -396,7 +396,11 @@ class AssetManagerTest extends Specification { given: 'A Git repository with files' def dir = tempDir.root dir.resolve('main.nf').text = "println 'Hello world'" - dir.resolve('nextflow.config').text = 'manifest { }' + dir.resolve('nextflow.config').text = ''' + manifest { + defaultBranch = 'master' + } + ''' dir.resolve('foo.nf').text = 'this is foo content' and: 'Initialize Git repository' @@ -658,34 +662,14 @@ class AssetManagerTest extends Specification { holder.manifest.getDefaultBranch() == 'master' } - def "should work with no defaultBranch"() { - given: - def config = ''' - manifest { - } - ''' - def dir = tempDir.getRoot() - dir.resolve('foo/bar').mkdirs() - dir.resolve('foo/bar/nextflow.config').text = config - dir.resolve('foo/bar/.git').mkdir() - dir.resolve('foo/bar/.git/config').text = GIT_CONFIG_TEXT - - when: - def holder = new AssetManager() - holder.build('foo/bar') - - then: - holder.manifest.getDefaultBranch() == 'master' - holder.manifest.getDefaultRevision() == 'master' - } - - def "should default to version tag if manifest version and no defaultBranch"() { + def "should handle development version with stable defaultRevision"() { given: def config = ''' - manifest { - version = '3.0.0' - } - ''' + manifest { + version = '2.3.0dev' + defaultRevision = '2.2.0' + } + ''' def dir = tempDir.getRoot() dir.resolve('foo/bar').mkdirs() dir.resolve('foo/bar/nextflow.config').text = config @@ -697,9 +681,9 @@ class AssetManagerTest extends Specification { holder.build('foo/bar') then: - holder.manifest.getVersion() == '3.0.0' - holder.manifest.getDefaultRevision() == '3.0.0' - holder.manifest.getDefaultBranch() == 'master' + holder.manifest.getVersion() == '2.3.0dev' + holder.manifest.getDefaultRevision() == '2.2.0' + holder.manifest.isDevelopmentVersion() == true } def "should handle commit hash in defaultRevision"() { @@ -707,6 +691,7 @@ class AssetManagerTest extends Specification { def config = ''' manifest { defaultRevision = '6b9515aba6c7efc6a9b3f273ce116fc0c224bf68' + version = '2.0.0' // Version should be ignored when defaultRevision is set } ''' def dir = tempDir.getRoot() @@ -720,30 +705,11 @@ class AssetManagerTest extends Specification { holder.build('foo/bar') then: + holder.manifest.getVersion() == '2.0.0' holder.manifest.getDefaultRevision() == '6b9515aba6c7efc6a9b3f273ce116fc0c224bf68' holder.manifest.getDefaultBranch() == 'master' } - @PendingFeature - def "should not warn if project uses a tag as a defaultBranch"() { - given: - def ENV = [FOO: '/something', NXF_DEBUG: 'true'] - - when: - new CmdRun(revision: 'xyz') - - then: - def warning = capture - .toString() - .readLines() - .findResults { line -> line.contains('WARN') ? line : null } - .join('\n') - and: - !warning - noExceptionThrown() - } - - @PendingFeature def "should handle development version with stable defaultRevision"() { given: def config = ''' @@ -768,7 +734,6 @@ class AssetManagerTest extends Specification { holder.manifest.isDevelopmentVersion() == true } - @PendingFeature def "should correctly compare development and release versions"() { given: def config = ''' @@ -802,10 +767,10 @@ class AssetManagerTest extends Specification { then: def warning = capture - .toString() - .readLines() - .findResults { line -> line.contains('WARN') ? line : null } - .join('\n') + .toString() + .readLines() + .findResults { line -> line.contains('WARN') ? line : null } + .join('\n') and: !warning noExceptionThrown()