Skip to content

Conversation

bentsherman
Copy link
Member

@bentsherman bentsherman commented Feb 14, 2024

Close #2723

Implements the strict config syntax in Nextflow, using the shared "compiler" module from the language server.

To use the strict config syntax, simply set NXF_SYNTAX_PARSER=v2 in your environment when running nextflow.

This config loader is much simpler and easier to read. There is no more dependence on metaclasses, instead there is a simple builder DSL for executing a config file and producing a map.

NOTE: While this PR uses Jitpack to load the shared module, this dependency should be inverted before the release of 25.04. That is, eventually the compiler module should reside in Nextflow and the language server should consume it.

Signed-off-by: Ben Sherman <[email protected]>
Copy link

netlify bot commented Feb 14, 2024

Deploy Preview for nextflow-docs-staging canceled.

Name Link
🔨 Latest commit e6b4048
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/67bdf93b441a7c0008156a33

@bentsherman bentsherman changed the title Add custom config parser Config parser Feb 15, 2024
Signed-off-by: Ben Sherman <[email protected]>
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Copy link
Member Author

Research updates:

  • Groovy compiler provides an interface to use a custom parser (in order to ease the transition from Antlr2 to Antlr4), this can be used to inject our custom parser without much hassle
  • Config schema effort can be pursued independently of the parser, see Config schema #4201
  • Might be able to use the Java security manager to prevent execution of unsafe code in the config file such as file I/O, subprocesses

@bentsherman
Copy link
Member Author

Here's an example to illustrate how the config parser "executes" the config file. Given this config:

foo.bar = 'hello'

foo {
  bar = 'hello'
}

includeConfig 'foobar.config'

The parser transforms this code into the following:

assign(['foo', 'bar'], 'hello')

block('foo', {
  assign(['bar'], 'hello')
}

includeConfig('foobar.config')

Basically each statement is translated to a DSL method (see ConfigDsl). This "hidden DSL" allows us to control how each statement is executed. In other words, the config parser + hidden DSL implement a simple interpreter for the Nextflow config language, separating the config language from the config execution.

I think this is a good model for how we might separate the Nextflow language from the runtime. Instead of hooking directly into Groovy methods and relying on Groovy MOP, metaclass, etc, we can introduce a hidden DSL which allows us to better control how a given statement (e.g. invoking a process in a workflow) is executed by the runtime.

@bentsherman
Copy link
Member Author

Java security manager is being removed. I will keep an eye out for other sandboxing techniques, but in the meantime, a simple solution is to provide a "safe" execution mode where assignment expressions are wrapped in string literals.

@bentsherman bentsherman changed the title Config parser Config parser (and loader) Mar 23, 2024
@ewels
Copy link
Member

ewels commented Apr 7, 2024

Really like the way that this is going, great work 👏🏻

Signed-off-by: Ben Sherman <[email protected]>
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman

This comment was marked as resolved.

Signed-off-by: Ben Sherman <[email protected]>

}

def 'should apply profiles in the order they were defined' () {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pditommaso this test is related to #1792 . We determined that config profiles are applied this way with the legacy parser. We could patch it, but I figure there's no point now, so just added a test to verify it's behavior.

Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Copy link
Member Author

@pditommaso tests are passing, let's review and merge

Copy link
Collaborator

@christopher-hakkaart christopher-hakkaart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor suggestion but otherwise the docs look great.

Co-authored-by: Chris Hakkaart <[email protected]>
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman

This comment was marked as resolved.

Signed-off-by: Paolo Di Tommaso <[email protected]>
Copy link
Member

@pditommaso pditommaso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome

@pditommaso pditommaso merged commit b019b0b into master Feb 25, 2025
10 checks passed
@pditommaso pditommaso deleted the ben-config-parser branch February 25, 2025 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

4 participants