Sorry for typos. I meant "If it’s not built within git or SCM"
> On Aug 15, 2019, at 9:23 AM, Ethan Li <[email protected]> wrote:
>
> I prefer to revert the change in VersionInfoMojo. These fields
> version-info.properties are introduced on purpose in
> https://github.com/apache/storm/pull/2844
> <https://github.com/apache/storm/pull/2844>
> https://github.com/apache/storm/pull/2891
> <https://github.com/apache/storm/pull/2891> so we should probably keep it.
>
> It makes sense to return unknown if it’s built within git or SCM as there is
> no other way to tell.
>
>
>> On Aug 15, 2019, at 9:12 AM, Derek Dagit <[email protected]
>> <mailto:[email protected]>> wrote:
>>
>> Well, the change seems to be to throw an IllegalArgumentException
>> instead of what was the default behavior: to return a String "Unknown".
>>
>> It is unclear how it would work outside of a Git working tree (as when
>> the source is built from a download), so it seems the easiest fix would
>> be to change the default labels to do nothing and revert to the previous
>> behavior—if we can convince checkstyle that it is OK.
>>
>> A better fix might be as you suggested. I like the idea of the build not
>> behaving differently depending on if it is executed within SCM, at least
>> if we can help it.
>>
>> In any case, I am inclined to think that this should be fixed before the
>> release, as it is a bug that prevents the code from building as per our
>> instructions.
>>
>> On Thu, Aug 15, 2019 at 02:08:17PM +0200, Stig Rohde Døssing wrote:
>>>
>>> As far as I can tell, the version plugin is supposed to populate these
>>> files
>>> https://github.com/apache/storm/blob/master/storm-client/src/resources/storm-client-version-info.properties
>>>
>>> <https://github.com/apache/storm/blob/master/storm-client/src/resources/storm-client-version-info.properties>
>>> and
>>> https://github.com/apache/storm/blob/master/storm-core/src/resources/storm-core-version-info.properties.
>>> I'm not sure why we need this file in both storm-client and storm-core, I'm
>>> not sure anything reads the storm-core file, so maybe we can delete it?
>>>
>>> When the build runs, those files are supposed to be populated with version
>>> info (commit hash, author info and so on) from the plugin. The version info
>>> is then read into
>>> https://github.com/apache/storm/blob/925422a5b5ad1c3329a2c2b44db460ae94f70806/storm-client/src/jvm/org/apache/storm/utils/VersionInfo.java.
>>> The VersionInfo file also loads the Storm version from the pom, where it's
>>> used to populate e.g. the version in Storm UI for Nimbus and Supervisors.
>>>
>>> It looks like this:
>>>
>>> version=2.0.0
>>> revision=2ba95bbd1c911d4fc6363b1c4b9c4c6d86ac9aae
>>> branch=(no branch)
>>> user=tgoetz
>>> date=2019-04-29T21:17Z
>>> url=https://git-wip-us.apache.org/repos/asf/storm.git
>>> srcChecksum=58779abf76d8a8bbd20a2bd5f466
>>>
>>> The only field I see read is the Storm version from the pom, I'm not sure
>>> we use the other stuff (e.g. commit hash, commit author). For 2.0.0
>>> something broke when splitting out storm-client, so the file for that
>>> module doesn't have all fields populated:
>>>
>>> version=2.0.0
>>> revision=${version-info.scm.commit}
>>> branch=${version-info.scm.branch}
>>> user=tgoetz
>>> date=${version-info.build.time}
>>> url=${version-info.scm.uri}
>>> srcChecksum=${version-info.source.md5}
>>>
>>> It's most likely just because storm-client is missing configuration to run
>>> the plugin when building.
>>>
>>> I think if we're not using these extra fields from git, we might as well
>>> drop the version info mojo. If we only need to set the "version" field, we
>>> don't need to invoke git during the build.
>>>
>>> If we do need these fields for something, we should just make the mojo set
>>> the fields to "Unknown" if it fails to invoke git (i.e. if SCM.NONE is
>>> set). I'm guessing what's happening for Derek is that he downloaded the
>>> sources and is trying to run the build with files that aren't part of a git
>>> repo. So when the mojo tries to do "git branch" it gets an error.
>>>
>>> What do you think?
>>>
>>> Den tor. 15. aug. 2019 kl. 06.02 skrev Ethan Li <[email protected]>:
>>>
>>>> Derek ran into this error when building from source:
>>>>
>>>> [ERROR] Failed to execute goal
>>>> org.apache.storm:storm-maven-plugins:2.1.0:version-info (version-info) on
>>>> project storm-core: java.lang.IllegalArgumentException: SCM NONE is not
>>>> supported -> [Help 1]
>>>>
>>>>
>>>> The log before this is
>>>>
>>>> [INFO] --- storm-maven-plugins:2.1.0:version-info (version-info) @
>>>> storm-core ---
>>>> [WARNING] [svn, info] failed with error code 1
>>>> [WARNING] [git, branch] failed with error code 128
>>>> [INFO] SCM: NONE
>>>>
>>>>
>>>> I compared with apache-storm-2.0.0 and found that it can pass although
>>>> there are similar WARNINGs:
>>>>
>>>> [INFO] --- storm-maven-plugins:2.0.0:version-info (version-info) @
>>>> storm-core ---
>>>> [WARNING] [svn, info] failed with error code 1
>>>> [WARNING] [git, branch] failed with error code 128
>>>> [INFO] SCM: NONE
>>>> [INFO] Computed MD5: 58779abf76d8a8bbd20a2bd5f466
>>>>
>>>> This is the PR that changed the behavior
>>>> https://github.com/apache/storm/pull/3061/files#diff-f1197661326b04376e94e3b25cc3eedcR197
>>>> <
>>>> https://github.com/apache/storm/pull/3061/files#diff-f1197661326b04376e94e3b25cc3eedcR197>.
>>>> I am not sure what this storm-maven-plugin should do though. Will need to
>>>> look into it. Or if anyone has better idea, please let me know.
>>>>
>>>>
>>>>
>>>>> On Aug 14, 2019, at 6:53 PM, P. Taylor Goetz <[email protected]> wrote:
>>>>>
>>>>> Nice work Ethan!
>>>>>
>>>>> I’ll try to take a look in the next few days.
>>>>>
>>>>> -Taylor
>>>>>
>>>>>> On Aug 14, 2019, at 2:13 PM, Ethan Li <[email protected]> wrote:
>>>>>>
>>>>>> This is a call to vote on releasing Apache Storm 2.1.0 (rc2)
>>>>>>
>>>>>> Full list of changes in this release:
>>>>>>
>>>>>>
>>>> https://dist.apache.org/repos/dist/dev/storm/apache-storm-2.1.0-rc2/RELEASE_NOTES.html
>>>>>>
>>>>>> The tag/commit to be voted upon is v2.1.0:
>>>>>>
>>>>>>
>>>> https://gitbox.apache.org/repos/asf?p=storm.git;a=tree;h=6c7adb8c7095cb718d5c49d07331034ae0e8434a;hb=f875619fc1017e401d65bd2900b195738f90e754
>>>>>>
>>>>>> The source archive being voted upon can be found here:
>>>>>>
>>>>>>
>>>> https://dist.apache.org/repos/dist/dev/storm/apache-storm-2.1.0-rc2/apache-storm-2.1.0-src.tar.gz
>>>>>>
>>>>>> Other release files, signatures and digests can be found here:
>>>>>>
>>>>>> https://dist.apache.org/repos/dist/dev/storm/apache-storm-2.1.0-rc2
>>>>>>
>>>>>> The release artifacts are signed with the following key:
>>>>>>
>>>>>> https://www.apache.org/dist/storm/KEYS
>>>>>>
>>>>>> The Nexus staging repository for this release is:
>>>>>>
>>>>>> https://repository.apache.org/content/repositories/orgapachestorm-1085/
>>>>>>
>>>>>> Please vote on releasing this package as Apache Storm 2.1.0.
>>>>>>
>>>>>> When voting, please list the actions taken to verify the release.
>>>>>>
>>>>>> This vote will be open for at least 72 hours.
>>>>>>
>>>>>> [ ] +1 Release this package as Apache Storm 2.1.0
>>>>>> [ ] 0 No opinion
>>>>>> [ ] -1 Do not release this package because...
>>>>>>
>>>>>> Thanks to everyone who contributed to this release.
>>>>>>
>>>>>> Ethan
>>>>
>>>>
>
