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

Fixed a NullPointerException if versionNamingPattern is not defined #622

Merged
merged 1 commit into from
Apr 21, 2015
Merged

Fixed a NullPointerException if versionNamingPattern is not defined #622

merged 1 commit into from
Apr 21, 2015

Conversation

greek1979
Copy link
Contributor

Hello everyone,

It seems as one major use case was overlooked while working on #605 and #606. What happens if Maven descriptor maintainer DOES NOT include or declare the versionNamingPattern property? Or leaves it blank. Judging by the way Maven behaves, the propery would evaluate to null, and cause a NullPointerException when accessing the value in VersionGenetator class constructor. Setting Parameter annotation defaultValue to empty parenthesis does not help much.

Hence there are two options. Either something is totally wrong with Maven 3.2.1 I am using (which could be the case after all), or the fix proposed here solves the issue at hand, without breaking anything.

@Shusshu
Copy link
Member

Shusshu commented Apr 21, 2015

That's weird because the default value is "" and your commit remove the default value...

I'll see if I can test this later or maybe someone else can try that

Could you try with the latest maven?

Edit:

Probable means that the Parameter with default value = "" means null....
I guess we can merge this. (I'll wait a bit if someone wants to chime in) @simpligility/android-maven-plugins-core-committers

@greek1979
Copy link
Contributor Author

Based on what I managed to learn about Maven (to be precise, Maven Plugin API), you are right about "Probable means that the Parameter with default value = "" means null". Setting default value to any empty string - nothing but whitespace - does translate into null. Hence defaultValue="" makes no sense and does not work.

UPDATE Tested with Apache Maven 3.1.1, 3.2.1, and finally with 3.3.1. No difference in Plugin API behavior; setting default value to empty string means null and AMP fails with NPE.

Shusshu added a commit that referenced this pull request Apr 21, 2015
Fixed a NullPointerException if versionNamingPattern is not defined
@Shusshu Shusshu merged commit 9194835 into simpligility:master Apr 21, 2015
@Shusshu
Copy link
Member

Shusshu commented Apr 21, 2015

Yes this is good.
@mosabua so there's a problem with 4.2.0. Should we make a 4.2.1 now?

@mosabua
Copy link
Member

mosabua commented Apr 21, 2015

Imho not worth it. It doesnt block anyone really. I would rather wait and get a few more other things in.

@greek1979 greek1979 deleted the version-generator-npe branch April 22, 2015 07:12
@xen0n
Copy link
Contributor

xen0n commented Apr 24, 2015

Uh-oh, my bad in not really test-running the fallback. You're right about the empty string behavior; removing the property did cause NPE, sorry about that.

Time to build more Java intuition...

@kingargyle
Copy link

Just got bit by this. Only way around it was to provide a magic regex expression in the Java regex syntax to get things passing again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants