Re: [josm-dev] JOSM Version check source (VersionTest macro) to troubleshoot strange version

2015-10-10 Thread Sebastiaan Couwenberg
On 10-10-15 00:01, Dirk Stöcker wrote:
> On Fri, 9 Oct 2015, Sebastiaan Couwenberg wrote:
> 
>>> No. As you mark it as Debian in the agent it's correct to strip the SVN
>>> text. This patch as far as I remember was designed in cooperation with
>>> me. For SVN we react different in tickets - We tell the user first to
>>> update to recent SVN version assuming the user can build the software
>>> himself. This is not the correct reply for Debian version, so the SVN
>>> should not be there. Simply change the patch to set "Debian" instead of
>>> "SVN", as I see that there is no patch in the GIT yet for this purpose.
>>
>> Based on the bug report and your changes to the VersionTest macro, it
>> seemed the patch may have been unneeded in the first place.
>>
>> I prefer the User-Agent without SVN substring anyway, so I'll happily
>> keep it for the Debian package.
>>
>> The Debian Build-Name change is currently only available in the
>> 00-build.patch on the stretch branch:
>>
>> https://anonscm.debian.org/cgit/pkg-grass/josm.git/tree/debian/patches/00-build.patch?h=stretch#n57
>>
> 
> Hmm, maybe these two together need some rework. If you simply change
> Is-Local-Build (above the Build-Name) to false the patch to Version.java
> can be dropped and manifest is more correct.

Settings Is-Local-Build to false goes against the guidelines from the
'How to create a build' documentation [0].

The Debian package is not an "official" release provided by
josm.openstreetmap.de for which Is-Local-Build: true is appropriate.

If the motivation for this change is to not need the Version.java
change, I don't think that's appropriate. I prefer keeping Version.java
change and correctly identifying the Debian package builds as a custom
build not provided by josm.openstreetmap.de.

[0]
https://josm.openstreetmap.de/wiki/DevelopersGuide/CreateBuild#TheREVISIONfile

> Removing SVN from the other line in build.xml is probably still required
> (we don't use build.xml but a makefile on the server). But I'd include
> that single change in your 00-build.patch for better readability.

The patches have different origins and authors, that's why I prefer to
keep them separate. I've also considered merging all build.xml changes
in 00-build.patch, but so far prefer to keep the separate patches.

Kind Regards,

Bas

-- 
 GPG Key ID: 4096R/6750F10AE88D4AF1
Fingerprint: 8182 DE41 7056 408D 6146  50D1 6750 F10A E88D 4AF1

___
josm-dev mailing list
josm-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/josm-dev


Re: [josm-dev] JOSM Version check source (VersionTest macro) to troubleshoot strange version

2015-10-10 Thread Vincent Privat
Speaking of Debian patches, I think you should also remove the "macosx"
folder, like what is done for the "windows" folder.
You may also remove from build.xml the following targets: mac, distmac,
distwin
You can also remove xmltask.jar from tools directory (it is only used to
build the OSX package)

Cheers,
Vincent

2015-10-10 0:01 GMT+02:00 Dirk Stöcker :

> On Fri, 9 Oct 2015, Sebastiaan Couwenberg wrote:
>
> No. As you mark it as Debian in the agent it's correct to strip the SVN
>>> text. This patch as far as I remember was designed in cooperation with
>>> me. For SVN we react different in tickets - We tell the user first to
>>> update to recent SVN version assuming the user can build the software
>>> himself. This is not the correct reply for Debian version, so the SVN
>>> should not be there. Simply change the patch to set "Debian" instead of
>>> "SVN", as I see that there is no patch in the GIT yet for this purpose.
>>>
>>
>> Based on the bug report and your changes to the VersionTest macro, it
>> seemed the patch may have been unneeded in the first place.
>>
>> I prefer the User-Agent without SVN substring anyway, so I'll happily
>> keep it for the Debian package.
>>
>> The Debian Build-Name change is currently only available in the
>> 00-build.patch on the stretch branch:
>>
>>
>> https://anonscm.debian.org/cgit/pkg-grass/josm.git/tree/debian/patches/00-build.patch?h=stretch#n57
>>
>
> Hmm, maybe these two together need some rework. If you simply change
> Is-Local-Build (above the Build-Name) to false the patch to Version.java
> can be dropped and manifest is more correct.
>
> Removing SVN from the other line in build.xml is probably still required
> (we don't use build.xml but a makefile on the server). But I'd include that
> single change in your 00-build.patch for better readability.
>
> Ciao
> --
> http://www.dstoecker.eu/ (PGP key available)
>
> ___
> josm-dev mailing list
> josm-dev@openstreetmap.org
> https://lists.openstreetmap.org/listinfo/josm-dev
>
___
josm-dev mailing list
josm-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/josm-dev