Fix staging of Path fields of included record types#7226
Open
robsyme wants to merge 1 commit into
Open
Conversation
When a typed process declares an input whose type is a record imported from another module, the implicit stageAs directives for the record's Path fields were not generated, so the fields were interpolated into the task script as raw source paths instead of staged paths. On a shared local filesystem the raw path still resolves, masking the bug, but on an object-store work dir (e.g. Fusion/S3) the task fails with 'No such file or directory'. The cause is the per-source analysis order in the script compiler: the entry script was resolved and converted to Groovy before the modules it includes, so the field types of an included record were still unresolved ClassNodes when ProcessToGroovyVisitorV2 inspected them to infer the implicit file inputs, and the Path fields were not recognized as such. Fix by tracking the include dependencies of each source file during module resolution and analyzing each source only after the modules it includes from, so that included types are fully resolved before they are used. Note: issue #7225 attributes the failure to the moduleBinaries feature flag, but the actual trigger is the record type being included from a separate module file; the flag is unrelated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Rob Syme <rob.syme@gmail.com>
✅ Deploy Preview for nextflow-docs-staging canceled.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7225
Problem
When a typed process (
nextflow.enable.types = true) declares an input whose type is a record included from another module file, the implicitstageAsdirectives for the record'sPathfields are not generated. The fields are interpolated into.command.shas raw source paths instead of staged paths. On a shared local filesystem the raw path still resolves, which masks the bug, but on an object-store work dir (e.g. Fusion/S3) the task fails withNo such file or directory.Isolating the variables with the reproducer shows the trigger is the record type being included from a separate file (the
nextflow.enable.moduleBinariesflag suspected in the original report is unrelated):types.nfmain.nfTaskPathCause
ScriptCompilerperforms resolution and Groovy conversion per source file, and the entry script is analyzed before the modules it includes. WhenProcessToGroovyVisitorV2inspects a record input to infer the implicit file inputs, the included record node is found (isRecordTypeis true) but its field types are still unresolved ClassNodes, soisPathTypereturns false and no stager is generated for thePathfields. The unstaged field then bypasses the staged-path substitution inTaskInputResolver.normalizeValueand reaches the task script as a raw source path.Solution
ModuleResolvernow records the include dependencies of each source file, returning existing source units on repeat encounters so the dependency graph is complete.ScriptCompileranalyzes each source file only after the modules it includes from, so included types are fully resolved before they are used. Guards handle include cycles and unparseable modules.Verification
ScriptLoaderV2Test."should stage path fields of an included record type"fails before the change (0 file inputs generated) and passes after (2 file inputs).TaskPathnames instead of raw paths.StackOverflowError, but this is pre-existing behavior at runtime module loading (IncludeDef.loadModuleV2), unchanged by this PR.🤖 Generated with Claude Code