From e1a0b240a3c438c9f289491a522b9a58535e5d19 Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Wed, 22 Dec 2021 05:59:52 +0100 Subject: [PATCH] Fix error reporting on missing script file #1951 --- .../main/groovy/nextflow/cli/CmdRun.groovy | 27 +++++++++++++++++++ .../groovy/nextflow/scm/AssetManager.groovy | 11 +++++++- .../groovy/nextflow/cli/CmdRunTest.groovy | 15 +++++++++++ .../nextflow/scm/AssetManagerTest.groovy | 10 +++++++ 4 files changed, 62 insertions(+), 1 deletion(-) diff --git a/modules/nextflow/src/main/groovy/nextflow/cli/CmdRun.groovy b/modules/nextflow/src/main/groovy/nextflow/cli/CmdRun.groovy index 9ff8d7a502..5a18d110bf 100644 --- a/modules/nextflow/src/main/groovy/nextflow/cli/CmdRun.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/cli/CmdRun.groovy @@ -338,6 +338,33 @@ class CmdRun extends CmdBase implements HubOptions { } protected ScriptFile getScriptFile(String pipelineName) { + try { + getScriptFile0(pipelineName) + } + catch (IllegalArgumentException | AbortOperationException e) { + if( e.message.startsWith("Not a valid project name:") && !guessIsRepo(pipelineName)) { + throw new AbortOperationException("Cannot find script file: $pipelineName") + } + else + throw e + } + } + + static protected boolean guessIsRepo(String name) { + if( FileHelper.getUrlProtocol(name) != null ) + return true + if( name.startsWith('/') ) + return false + if( name.startsWith('./') || name.startsWith('../') ) + return false + if( name.endsWith('.nf') ) + return false + if( name.count('/') != 1 ) + return false + return true + } + + protected ScriptFile getScriptFile0(String pipelineName) { assert pipelineName /* diff --git a/modules/nextflow/src/main/groovy/nextflow/scm/AssetManager.groovy b/modules/nextflow/src/main/groovy/nextflow/scm/AssetManager.groovy index 30e1033c78..ea40e03d09 100644 --- a/modules/nextflow/src/main/groovy/nextflow/scm/AssetManager.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/scm/AssetManager.groovy @@ -250,10 +250,19 @@ class AssetManager { String resolveName( String name ) { assert name + // + // check if it's a repository fully qualified URL e.g. https://github.com/foo/bar + // def project = resolveNameFromGitUrl(name) if( project ) return project + // + // otherwise it must be a canonical repository name e.g. user/project + // + if( ['./','../', '/' ].any(it->name.startsWith(it)) ) + throw new AbortOperationException("Not a valid project name: $name") + def parts = name.split('/') as List def last = parts[-1] if( last.endsWith('.nf') || last.endsWith('.nxf') ) { @@ -459,7 +468,7 @@ class AssetManager { result = (ConfigObject)config.manifest } catch( Exception e ) { - throw new Exception("Project config file is malformed -- Cause: ${e.message ?: e}", e) + throw new AbortOperationException("Project config file is malformed -- Cause: ${e.message ?: e}", e) } // by default return an empty object diff --git a/modules/nextflow/src/test/groovy/nextflow/cli/CmdRunTest.groovy b/modules/nextflow/src/test/groovy/nextflow/cli/CmdRunTest.groovy index 7c00a9df17..be400a90d5 100644 --- a/modules/nextflow/src/test/groovy/nextflow/cli/CmdRunTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/cli/CmdRunTest.groovy @@ -252,4 +252,19 @@ class CmdRunTest extends Specification { then: cmd.getDisableJobsCancellation() == true } + + @Unroll + def 'should guss is repo' () { + expect: + CmdRun.guessIsRepo(PATH) == EXPECTED + + where: + EXPECTED | PATH + true | 'http://github.com/foo' + true | 'foo/bar' + and: + false | 'script.nf' + false | '/some/path' + false | '../some/path' + } } diff --git a/modules/nextflow/src/test/groovy/nextflow/scm/AssetManagerTest.groovy b/modules/nextflow/src/test/groovy/nextflow/scm/AssetManagerTest.groovy index e418bda3ec..fd1ea6dce4 100644 --- a/modules/nextflow/src/test/groovy/nextflow/scm/AssetManagerTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/scm/AssetManagerTest.groovy @@ -130,6 +130,16 @@ class AssetManagerTest extends Specification { then: thrown(AbortOperationException) + when: + result = manager.resolveName('../blast/script.nf') + then: + thrown(AbortOperationException) + + when: + result = manager.resolveName('./blast/script.nf') + then: + thrown(AbortOperationException) + }