Skip to content

Commit 2c11fec

Browse files
pditommasoclaude
andcommitted
Fix additional code issues in module system
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
1 parent 34426ea commit 2c11fec

10 files changed

Lines changed: 33 additions & 25 deletions

File tree

modules/nextflow/src/main/groovy/nextflow/cli/module/CmdModuleInfo.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ class CmdModuleInfo extends CmdBase {
119119
throw new AbortOperationException("No release information available for ${reference}")
120120
}
121121
if( !release.metadata ) {
122-
log.info("No metadata found for $reference ${release.version ? "($release.version)" : ''}")
122+
throw new AbortOperationException("No metadata found for ${reference}${release.version ? " (${release.version})" : ''}")
123123
}
124124
def moduleUrl = buildModuleUrl(registryConfig.url, reference, release.version)
125125
if( !output || output == 'text' ) {

modules/nextflow/src/main/groovy/nextflow/cli/module/CmdModulePublish.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ class CmdModulePublish extends CmdBase {
260260
final localStorage = new ModuleStorage(root ?: Paths.get('.').toAbsolutePath().normalize())
261261

262262
if (!localStorage.isInstalled(ref)){
263-
throw new AbortOperationException("No module diretory found for $module")
263+
throw new AbortOperationException("No module directory found for $module")
264264
}
265265
useModuleReference = true
266266
return localStorage.getModuleDir(ref)

modules/nextflow/src/main/groovy/nextflow/module/ModuleInfo.groovy

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,13 @@ class ModuleInfo {
9494
* Load all properties from the .module-info file in the module directory
9595
*
9696
* @param moduleDir The module directory path
97-
* @return The checksum, or null if file doesn't exist
97+
* @return Map of properties, or null if file doesn't exist
9898
*/
9999
static Map<String, String> load(Path moduleDir) {
100100
def moduleInfoFile = moduleDir.resolve(MODULE_INFO_FILE)
101101
if( !Files.exists(moduleInfoFile) ) {
102102
log.debug("Module file $moduleInfoFile not found")
103-
return [:]
103+
return null
104104
}
105105
def props = new Properties()
106106
moduleInfoFile.withInputStream { is -> props.load(is) }

modules/nextflow/src/main/groovy/nextflow/module/ModuleReference.groovy

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,7 @@ import java.util.regex.Pattern
3131
@EqualsAndHashCode
3232
class ModuleReference {
3333

34-
// Pattern allows: scope with letters/digits/hyphens/dots/underscores, name segments separated by slashes (no trailing slash)
35-
// Scope: starts with letter/digit, followed by letters/digits/dots/underscores/hyphens
36-
// Name: one or more segments (each starting with letter, followed by letters/digits/underscores/hyphens), separated by slashes
34+
// Must stay in sync with ModuleResolver.REMOTE_MODULE_PATTERN in nf-lang
3735
private static final Pattern MODULE_NAME_PATTERN = ~/^([a-z0-9][a-z0-9._\-]*)\/([a-z][a-z0-9._\-]*(?:\/[a-z][a-z0-9._\-]*)*)$/
3836

3937
final String scope

modules/nextflow/src/main/groovy/nextflow/module/ModuleRegistryClient.groovy

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,9 @@ class ModuleRegistryClient {
276276
}
277277

278278
// Write response body to file
279-
Files.copy(response.body(), targetPath)
279+
try( def body = response.body() ) {
280+
Files.copy(body, targetPath)
281+
}
280282
log.debug "Downloaded module to: ${targetPath}"
281283

282284
validateDownloadIntegrity(response, uri, targetPath, name, version)

modules/nextflow/src/main/groovy/nextflow/module/ModuleStorage.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ class ModuleStorage {
228228
boolean removeModule(ModuleReference reference, boolean force) {
229229
final installed = getInstalledModule(reference)
230230
if( !installed )
231-
return
231+
return false
232232
final integrity = installed.integrity
233233
if( integrity == ModuleIntegrity.NO_REMOTE_MODULE && !force ) {
234234
throw new AbortOperationException(

modules/nextflow/src/test/groovy/nextflow/module/ModuleInfoTest.groovy

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ class ModuleInfoTest extends Specification {
169169
result['registryUrl'] == 'http://registry.com'
170170
}
171171

172-
def 'should return empty map when loading all from non-existent file'() {
172+
def 'should return null when loading all from non-existent file'() {
173173
given:
174174
def moduleDir = tempDir.resolve('module')
175175
Files.createDirectories(moduleDir)
@@ -178,6 +178,6 @@ class ModuleInfoTest extends Specification {
178178
def result = ModuleInfo.load(moduleDir)
179179

180180
then:
181-
result == [:]
181+
result == null
182182
}
183183
}

modules/nextflow/src/test/groovy/nextflow/module/ModuleStorageTest.groovy

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -507,19 +507,21 @@ class ModuleStorageTest extends Specification {
507507
new GZIPOutputStream(fos).withCloseable { gzos ->
508508
new TarArchiveOutputStream(gzos).withCloseable { tos ->
509509
// Add all files from tempModuleDir
510-
Files.walk(tempModuleDir).each { Path path ->
511-
if (Files.isRegularFile(path)) {
512-
// Get relative path
513-
def relativePath = tempModuleDir.relativize(path).toString()
510+
Files.walk(tempModuleDir).withCloseable { stream ->
511+
stream.each { Path path ->
512+
if (Files.isRegularFile(path)) {
513+
// Get relative path
514+
def relativePath = tempModuleDir.relativize(path).toString()
514515

515-
// Create tar entry
516-
def entry = new TarArchiveEntry(path.toFile(), relativePath)
517-
tos.putArchiveEntry(entry)
516+
// Create tar entry
517+
def entry = new TarArchiveEntry(path.toFile(), relativePath)
518+
tos.putArchiveEntry(entry)
518519

519-
// Write file content
520-
Files.copy(path, tos)
520+
// Write file content
521+
Files.copy(path, tos)
521522

522-
tos.closeArchiveEntry()
523+
tos.closeArchiveEntry()
524+
}
523525
}
524526
}
525527
}

modules/nf-commons/src/main/nextflow/config/RegistryConfig.groovy

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ class RegistryConfig implements ConfigScope {
5555
RegistryConfig(Map opts) {
5656
final urlObject = opts.url ?: [DEFAULT_REGISTRY_URL]
5757
if (urlObject instanceof Collection<String>)
58-
this.url = urlObject as Collection<String>
58+
this.url = (urlObject as Collection<String>).collect { it.replaceAll(/\/+$/, '') }
5959
else
60-
this.url = [urlObject.toString()]
60+
this.url = [urlObject.toString().replaceAll(/\/+$/, '')]
6161
this.apiKey = opts.apiKey as String
6262
}
6363

modules/nf-lang/src/main/java/nextflow/script/control/ModuleResolver.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,17 @@ private SourceUnit resolveInclude(IncludeNode node, SourceUnit sourceUnit, Funct
9797
return includeSource;
9898
}
9999

100+
/**
101+
* Module name pattern matching the canonical format used by ModuleReference.
102+
* Scope: lowercase alphanumeric with dots/underscores/hyphens.
103+
* Name: one or more slash-separated segments, each lowercase alphanumeric with dots/underscores/hyphens.
104+
*/
105+
static final String REMOTE_MODULE_PATTERN = "^[a-z0-9][a-z0-9._\\-]*/[a-z][a-z0-9._\\-]*(/[a-z][a-z0-9._\\-]*)*$";
106+
100107
static boolean isRemoteModule(String source) {
101108
if( source.startsWith("/") || source.startsWith("./") || source.startsWith("../") )
102109
return false;
103-
// Must match scope/name pattern: scope is lowercase alphanumeric with dots/underscores/hyphens
104-
return source.matches("^[a-z0-9][a-z0-9._\\-]*/[a-z][a-z0-9._\\-]*(/[a-z][a-z0-9._\\-]*)*$");
110+
return source.matches(REMOTE_MODULE_PATTERN);
105111
}
106112

107113
private static URI getIncludeUri(Path parent, String source) {

0 commit comments

Comments
 (0)