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

overrideUrl doesn't work #331

Closed
digulla opened this issue May 9, 2019 · 3 comments · May be fixed by marcus-nl/camunda-bpm-platform#3
Closed

overrideUrl doesn't work #331

digulla opened this issue May 9, 2019 · 3 comments · May be fixed by marcus-nl/camunda-bpm-platform#3

Comments

@digulla
Copy link

@digulla digulla commented May 9, 2019

There is a bug in the code which processes the overrideUrl in org.codehaus.mojo.license.LicenseMojoUtils.prepareUrl(String, File, String, File, String):

    final Path basedirPath = basedir.toPath();

    if ( url != null && UrlRequester.isStringUrl( url ) )
    {
        return basedirPath.toUri().toString();
    }

    final Path defaultPath = basedirPath.resolve( defaultFilePath );

should be

    if ( url != null && UrlRequester.isStringUrl( url ) )
    {
        return url;
    }

    final Path basedirPath = basedir.toPath();
    final Path defaultPath = basedirPath.resolve( defaultFilePath );
@ppalaga
Copy link
Contributor

@ppalaga ppalaga commented May 9, 2019

A couple of unit tests under https://github.com/mojohaus/license-maven-plugin/tree/master/src/test/java/org/codehaus/mojo/license would be highly welcome that would cover not only this broken case but also the cases that work properly now.

@digulla
Copy link
Author

@digulla digulla commented May 9, 2019

Can you please include the fix in 1.21? I tested it locally and without the fix, the functionality is completely broken for us.

You can then leave this issue open to add the unit tests later or create another issue for the tests.

@ppalaga
Copy link
Contributor

@ppalaga ppalaga commented May 9, 2019

Can you please include the fix in 1.21?

I am ready to review a PR from you and I will require the tests to merge it. The tests are for your own safety, so that it cannot happen that somebody breaks your stuff next day after the fix was merged.

Once the PR is merged, I have no problem to release promptly.

digulla pushed a commit to digulla/license-maven-plugin that referenced this issue May 10, 2019
@ppalaga ppalaga changed the title overrideUrl doesn't work (with fix) overrideUrl doesn't work May 10, 2019
@ppalaga ppalaga closed this in cd63928 May 12, 2019
ppalaga added a commit that referenced this issue May 12, 2019
@ppalaga ppalaga added this to the 2.0.0 milestone Jun 22, 2019
saberduck added a commit to SonarSource/parent-oss that referenced this issue Mar 13, 2020
I encountered an issue using overrideUrl with 1.16 which is fixed in 2.0.0 . See mojohaus/license-maven-plugin#331
tomverin added a commit to SonarSource/parent-oss that referenced this issue Mar 16, 2020
I encountered an issue using overrideUrl with 1.16 which is fixed in 2.0.0 . See mojohaus/license-maven-plugin#331
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants