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

Introduce workspace parameter #887

Open
mathieu opened this issue Jan 15, 2025 · 2 comments
Open

Introduce workspace parameter #887

mathieu opened this issue Jan 15, 2025 · 2 comments
Assignees
Labels
in:core MLP core module is:feature New feature todo Accepted items from the backlog which can be worked on

Comments

@mathieu
Copy link
Contributor

mathieu commented Jan 15, 2025

          > @mathieucarbou , I'll need some more context on your vision of the `<workspace>`. What does it do compared to LicenceSet's `<includes>` and `<excludes>` ?

workspace would define the global workspace to work one

  • workspace.baseDir == project root directory
  • workspace.includes defaulted to null
  • workspace.excludes defaulted to null

This concept exists already but is implicit:

  • the workspace.baseDir is currently defaultBasedir,
  • we do not constrain to any include (the whole workspace is visible)
  • and we do not constrain to some excludes (nothing is excluded by default).

The idea is not make it explicitly configurable.

LicenceSet are used to "split" the workspace into several groups of file and assigning them a license. So they inherit the workspace baseDir, includes, excludes. licensSet.baseDir can be overridden but must be a sub directory of the workspace (@hazendaz : I don't know if we make sure of that somewhere, but with the workspace concept we will be)

As I see things for includes/excludes:

  • if workspace.includes is defined in POM, then it will be added to all licenseSet.includes
  • if workspace.excludes is defined in POM, then it will be added to all licenseSet.excludes
  • if workspace.baseDir is defined in POM, then it will be used to set licenseSet.baseDir (if not set) - like what happens now with defaultBasedir, and if licenseSet.baseDir is set, we have to check that it is same or a subfolder of workspace.baseDir.

For backward compat', defaultBasedir must be marked deprecated, without any default now ,and if set by the user, its value will fill "workspace.baseDir". The property license.basedir should be moved to workspace.baseDir

In the case of a commit hook, then the only property to add would be:

-D license.workspace.includes=<list of file paths>

This behind will be transferred to licenseset.includes and will have at the end the same effect of my comment in this PR: #878 (comment)

Also, this "design" does not change the scanner logic, which is still taking place but based on a refinement of the baseDir / includes / excludes.

@mathieucarbou @hazendaz @frawa : let me know what you think of that ;-)

Originally posted by @mathieucarbou in #876 (comment)

@mathieucarbou mathieucarbou added todo Accepted items from the backlog which can be worked on is:feature New feature in:core MLP core module labels Jan 15, 2025
mathieu added a commit to mathieu/license-maven-plugin that referenced this issue Jan 15, 2025
@mathieu
Copy link
Contributor Author

mathieu commented Jan 15, 2025

@mathieucarbou, I've an issue with the following requirement

if workspace.baseDir is defined in POM, then it will be used to set licenseSet.baseDir (if not set) - like what happens now with defaultBasedir, and if licenseSet.baseDir is set, we have to check that it is same or a subfolder of workspace.baseDir.

The subfolder part makes LicenseSetTest fail since the defaultBasedir is not a parent of the other licenseSet's basedirs. Looks like a regression or a breaking change.

Any hint of how you'd like this to be handled ?

  • Move test file around
  • Change test logic

#888 shows the current initial state of the updates

@mathieucarbou
Copy link
Owner

The subfolder part makes LicenseSetTest fail since the defaultBasedir is not a parent of the other licenseSet's basedirs. Looks like a regression or a breaking change.

Any hint of how you'd like this to be handled ?

yes that's what I thought also that we might have an issue here. In theory, the way the plugin works, it should not tamper with files outside o its working directory. We didn't put this protection in place before, I think this is conventionally the case. I cannot imagine any use case where we want that...

I would update the test to apply this constraint. If someone comes with a valid reason to do so, then we could introduce a flag in the workspace object to allow the plugin to work on files outside of it.

@hazendaz : what do you think ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in:core MLP core module is:feature New feature todo Accepted items from the backlog which can be worked on
Projects
None yet
Development

No branches or pull requests

3 participants