nextflow-io / nextflow

A DSL for data-driven computational pipelines
http://nextflow.io
Apache License 2.0
2.61k stars 606 forks source link

Postfixing `+` in `nextflow.version.matches()` is not working properly #5016

Closed HarryHung closed 1 month ago

HarryHung commented 1 month ago

Bug report

Expected behavior and actual behavior

The following two code blocks should work the same (i.e. it should not go inside of the if block) if the Nextflow executable is at 23.10 or newer.

Version 1

nextflowMinVersion = '23.10'
if( !nextflow.version.matches("${nextflowMinVersion}+") ) {
    log.error("Requires Nextflow version ${nextflowMinVersion} or greater -- You are running version $nextflow.version") 
    System.exit(1)
}

Version 2

nextflowMinVersion = '23.10'
if( !nextflow.version.matches(">=${nextflowMinVersion}") ) {
    log.error("Requires Nextflow version ${nextflowMinVersion} or greater -- You are running version $nextflow.version") 
    System.exit(1)
}

In 23.xx, both versions work fine for me. But after I have upgraded the executable to 24.04.1, version 1 no longer works properly and go into the if block.

Steps to reproduce the problem

With the following content in test.nf

nextflowMinVersion = '23.10' 
if( !nextflow.version.matches("${nextflowMinVersion}+") ) {
    log.error("The pipeline requires Nextflow version ${nextflowMinVersion} or greater -- You are running version $nextflow.version") 
    System.exit(1)
}

println("Done")

By running ./nextflow run test.nf with...

Program output

As mentioned above

Environment

bentsherman commented 1 month ago

The root cause seems to be when the major version is greater but the minor version isn't. I'm surprised this bug wasn't noticed before! Looking into a patch

bentsherman commented 1 month ago

@pditommaso now I'm confused about the meaning of + postfix. I assumed it was equivalent to >= prefix but looking at the test cases there are some slight differences:

        '1.2'       | '1.2.+'       | true
        '1.3'       | '1.2.+'       | false // ???
        '2.3.1b'    | '2.3.+'       | true
        '2.4'       | '2.3.+'       | false // ???
        '1.2'       | '1.+'         | true

Apparently the plus works like >= except when you do .+, then it behaves somewhat differently. Is there some standard that we are following here? I know the version is supposed to be a semantic version but I don't see anything in that standard for specific comparison operators like +, only how to compare versions in general.

pditommaso commented 1 month ago

1.2.+ should allow 1.2.1, 1.2.2 but not 1.3.x

HarryHung commented 1 month ago

Doc mentions

postfixed with the + operator to specify a minimum version requirement

Shouldn't 1.2.+ allows 1.3.x as well?

pditommaso commented 1 month ago

1.2.+ != 1.2+ 😈

bentsherman commented 1 month ago

Then this behavior needs to be documented. @HarryHung I think the + postfix is behaving as intended, so you should just use >= if you only need a minimum requirement

HarryHung commented 1 month ago

Sure can do, but if this is the intended behaviour, I would suggest updating the docuementation:

postfixed with the + operator to specify a minimum version requirement. For example:

if( !nextflow.version.matches('21.04+') ) {
    println "This workflow requires Nextflow version 21.04 or greater -- You are running version $nextflow.version"
    exit 1
}

Because it seems both the wordings and the example do not align well with its intended behaviour?

Thanks!