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.

Sign up for GitHub

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

Allow to override useNativeGit from command line #433

Merged
merged 2 commits into from
Aug 25, 2019
Merged

Allow to override useNativeGit from command line #433

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b7e9ca
Allow to override useNativeGit from command line
alexott Aug 18, 2019
1b56f0d
Re-implement as per suggestion
alexott Aug 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/using-the-plugin.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,8 @@ It's really simple to setup this plugin; below is a sample pom that you may base

Although this should usually give your build some performance boost, it may randomly break if you upgrade your git version and it decides to print information in a different format suddenly.
As rule of thumb, keep using the default `jgit` implementation (keep this `false`) until you notice performance problems within your build (usually when you have *hundreds* of maven modules).

With version *3.0.2* you can also control it using the commandline option `-Dmaven.gitcommitid.nativegit=true`
-->
<useNativeGit>false</useNativeGit>

Expand Down
7 changes: 6 additions & 1 deletion src/main/java/pl/project13/maven/git/GitCommitIdMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,14 @@ public class GitCommitIdMojo extends AbstractMojo {
* Set this to {@code 'true'} to use native Git executable to fetch information about the repository.
* It is in most cases faster but requires a git executable to be installed in system.
* By default the plugin will use jGit implementation as a source of information about the repository.
*
* NOTE / WARNING:
* Do *NOT* set this property inside the configuration of your plugin, so it would be
* possible to override it from command line.
*
* @since 2.1.9
*/
@Parameter(defaultValue = "false")
@Parameter(property = "maven.gitcommitid.nativegit", defaultValue = "false")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately I don't think this is working with maven and I'm still convinced that this is still utterly broken in maven.

The NOTE/WARNING you have introduced (copy and pasted from one of the other setting?) refers to #315 where a similar problem was outlined for the skip property.

Essentially the problem here is that maven somehow does not respect the command-line option and will use the option configured in the pom (if available). However whenever a user has configured the setting in the pom the command-line option should overwrite that pom setting (IMHO that is at least how maven behaves). AFAIK this is not happening and is only possible when we need to introduce an additional property.

This could look like something like this:

  /** Set this to {@code 'true'} to use native Git executable to fetch information about the repository.
   * It is in most cases faster but requires a git executable to be installed in system.
   * By default the plugin will use jGit implementation as a source of information about the repository.
   *
   * @since 2.1.9
   */
  @Parameter(defaultValue = "false")
  boolean useNativeGit;


  /**
   * Set this to {@code 'true'} to use native Git executable to fetch information about the repository via commandline.
   * NOTE / WARNING:
   * Do *NOT* set this property inside the configuration of your plugin.
   * Please read
   * https://github.com/git-commit-id/maven-git-commit-id-plugin/issues/315
   * to find out why.
   * @since 3.0.2
   */
  @Parameter(property = "maven.gitcommitid.nativegit", defaultValue = "false")
  private boolean useNativeGitViaCommandLine;

Certainly whenever this plugin decided to run the native code it should run native whenever useNativeGitViaCommandLine or useNativeGit is true.

Why plugins are forced to do this this way is beyond me....and I would call it broken and not 'expected'...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for advice - I've re-implemented it as you suggested. The new implementation checks, if the given property is set, and then use the value provided - this allows to overwrite setting even if useNativeGit is explicitly set to true in the project - for example, when I don't have Git installed on my machine.

boolean useNativeGit;

/**
Expand Down