Skip to content
Permalink
Browse files

Fix `from` sometimes failing to ensure specified input is resolved in…

… preference to other available inputs preceding it in the pipeline

commit fc9ada8
Author: Simon Sadedin <ssadedin@gmail.com>
Date:   Tue Jun 11 15:29:55 2019 +1000

    fix incorrect declared type

commit 886472f
Author: Simon Sadedin <ssadedin@gmail.com>
Date:   Tue Jun 11 14:13:27 2019 +1000

    add missing change causing regression failures

commit 0512123
Author: Simon Sadedin <ssadedin@gmail.com>
Date:   Tue Jun 11 14:01:09 2019 +1000

    limit forced resolution of inputs to when inside a from clause to prevent side effects

commit dc075df
Author: Simon Sadedin <ssadedin@gmail.com>
Date:   Tue Jun 11 13:03:50 2019 +1000

    ignore errors if speculative eval of pipeline input fails

commit b6347eb
Author: Simon Sadedin <ssadedin@gmail.com>
Date:   Tue Jun 11 12:16:39 2019 +1000

    force resolution of input values when an output is referenced

    needed because groovy is invoking toString() in a second pass
    after evaluating all variables when building an exec string.
    This means that $output can be fully resolved even while
    $input is still pending resolution.

commit 3e1f934
Author: Simon Sadedin <ssadedin@gmail.com>
Date:   Mon Jun 10 12:49:06 2019 +0000

    fix os dependent behavior of cp causing test failures

commit b295654
Author: Simon Sadedin <ssadedin@gmail.com>
Date:   Mon Jun 10 21:13:00 2019 +1000

    fix regressions from from() fix, add better fix for original problem

commit b693d3e
Author: Simon Sadedin <ssadedin@gmail.com>
Date:   Mon Jun 10 19:30:38 2019 +1000

    adjust test to better reflect fix

commit 42a3f44
Author: Simon Sadedin <ssadedin@gmail.com>
Date:   Mon Jun 10 19:30:10 2019 +1000

    test fix for incorrect input resolution after from
  • Loading branch information...
ssadedin committed Jun 11, 2019
1 parent 736c33b commit 5799ee185247023379de82b2ecacc7a88820acd8
@@ -541,7 +541,24 @@ class PipelineContext {

// If an input property was referenced, compute the default from that instead
List<PipelineFile> allResolved = (List<PipelineFile>)allInputs.collect { Map.Entry<Integer,PipelineInput> e ->
((PipelineInput)e.value).resolvedInputs

PipelineInput resolvedInputs = e.value

// Because an output is being referenced, we need to force the input to resolve
// (it may not have resolved because Groovy does not invoke toString() upon reference, only
// after all variables have been resolved). This is only necessary after a `from` is
// applied because it resets the inputs into a state where previous resolution may have been
// lost
if(forceResolve) {
try {
resolvedInputs.toString()
}
catch(Exception exIgnore) {
// Ignore
}
}

return ((PipelineInput)e.value).resolvedInputs
}.flatten()

if(!allResolved) {
@@ -2632,6 +2649,8 @@ class PipelineContext {
return new PipelineFile(pathValue, defaultStorage)
}

boolean forceResolve = false

/**
* Executes the given closure with the inputs replaced with the specified
* inputs, and then restores them afterwards. If the closure passed is null,
@@ -2652,10 +2671,12 @@ class PipelineContext {
allResolvedInputs.addAll(this.getInput().resolvedInputs)
this.@input = oldInputs
this.@inputWrapper = null
this.forceResolve = false
}

this.@inputWrapper = null
if(body != null) {
this.forceResolve = true
def result = body()
resetInputs()
return result
@@ -451,11 +451,23 @@ class PipelineInput {
reverseOutputs.add(originalInputs)
}

// Add an initial stage that represents the current input to this stage. This way
// if the from() spec is used and matches the actual inputs then it will go with those
// rather than searching backwards for a previous match
log.info "Finally, add my actual input as the first thing to check: " + this.input
reverseOutputs.add(0,this.@input)
List inputInputs = []
PipelineInput root = this
while(root.parent != null) {
inputInputs << root.input
root = root.parent
}
inputInputs << root.input

// Add an initial stage that prioritises the inputs provided to or resolved
// by this PipelineInput already. This way if the from() spec is used and matches
// then it will go with those rather than searching backwards for a previous match
for(List inps in inputInputs.reverse()) {
if(inps) {
log.info "Add input from PipelineInput resolution chain: " + inps
reverseOutputs.add(0,inps)
}
}

return reverseOutputs
}
@@ -77,7 +77,7 @@ class PipelineOutput {
* Used when a choice between two possible output references is possible,
* to use the one best associated with actual resolved input
*/
List<String> resolvedInputs = []
List<PipelineFile> resolvedInputs = []

/**
* If a filter is executing, a list of the file extensions that were available
@@ -2,6 +2,6 @@ source ../testsupport.sh

run test.txt

grep -q 'test.txt -> something.txt' test.out || err 'File specified by from not used as default input'
grep -q 'test.txt.*->.*something.txt' test.out || err 'File specified by from not used as default input'

true
@@ -2,6 +2,6 @@ source ../testsupport.sh

run test.foo.txt test.bar.txt

grep -q 'test.bar.there.bam correct=test.bar.there.bam' test.out || err "Incorrect input used: does not correspond to from"
grep -q 'test.bar.there.bam.*->.*' test.out || err "Incorrect input used: does not correspond to from"

true
@@ -14,11 +14,11 @@ world = {

def correctBam = 'test.bar.there.bam'

from(correctBam) produce('bar.xml') {
exec """
echo "branch $branch.name: create $output.xml from $input.hello.vcf + $input.there.bam correct=$correctBam => $output.xml"
println "The correct BAM is $correctBam"

cat $input.hello.vcf $input.there.bam > $output.xml
from(correctBam) {
exec """
cp -v $input.there.bam $output.xml
"""
}
}

0 comments on commit 5799ee1

Please sign in to comment.
You can’t perform that action at this time.