Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recommend using from over setFrom in general #3

Open
lacasseio opened this issue May 11, 2022 · 3 comments
Open

Recommend using from over setFrom in general #3

lacasseio opened this issue May 11, 2022 · 3 comments
Labels

Comments

@lacasseio
Copy link
Member

This should most likely reside in "working with source files" chapter.

It's important to note the difference between setFrom and from. The first override while the second appends. It's also important to understand configuration order. We generally recommend using from and reserve setFrom only when necessary.

application {
    cSources.from(...)
}

//later
application {
    cSources.setFrom(...)
}

The first layout is overridden and lost. Nokee will try its best to track this information and report them to the user.

@lacasseio
Copy link
Member Author

I think the difference between those two APIs should be under nokee.fyi site. We should also point directly in the code of the other sample to the FYI page so that if users copy and paste the code, the consideration would also follow thus avoiding any issues regarding the correct API to use.

There is two consideration with these APIs: 1) the intention and 2) the ordering.

The intention to completely replace the default values because of unwanted files would correctly point to the use setFrom. Unwanted files could mean removing files from a common location thus requiring a more picky declaration. It's usually preferable to add the correct files at the source, see layering FileCollection. However, the intention of ensuring a clean FileCollection is mostly a bad reason for using setFrom. By clean FileCollection, we mean removing paths that point to nothing. A FileCollection will ignore missing files/directories upon resolution, thus using the setFrom API may cause more harm with regard to ordering. It's important to understand the default values and how they affect your resulting file selection. Each plugin reference chapter should include this information to allow a clear decision.

The ordering is especially important when it comes to playing nice with other plugins. Most plugins would most likely append additional paths to the FileCollection (e.g. from). If your override logic comes after the plugin's append, you would effectively change the plugin's intention and possibly break what the plugin is trying to accomplish. It's important to understand how the other plugins are manipulating the target FileCollection and ensure we play nice with them. Plugin author should consider layering FileCollection (see next section).

Layering FileCollection

Plugin authors should focus on layering additional ConfigurableFileCollection to ensure finer grain control over the resulting FileCollection. For example,

// Plugin A
def myCommonSources = objects.fileCollection().from('some-common-location')
extensions.add("commonSources", myCommonSources)
application {
    cSources.from(myCommonSources)
}
// Plugin B
def protoTask = tasks.register('compileProto', ProtobufCompile)
application {
    cSources.from(protoTask)
}
// Convention plugin
commonSources.setFrom(fileTree('src/common') { include '**/*.c' })

We can override only the commons sources and avoid breaking the wiring for the compileProto task.

@Ladalz
Copy link

Ladalz commented May 16, 2022

I need more explaining for this one or wait later TODO

@lacasseio
Copy link
Member Author

I think a drawing would help a lot with understanding the layering of FileCollection in terms of how everything comes together. For the ordering, a timeline with code could also help with understanding the danger. Let's have a chat about that.

@lacasseio lacasseio transferred this issue from nokeedev/gradle-native May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants