[rfc][icedtea-web] Improvements for running the html-gen.sh script manually

[Jiri Vanek]

On 04/02/2015 03:33 AM, Jacob Wisor wrote:
> Am 01.04.2015 um 17:47 schrieb Jiri Vanek:
>> On 04/01/2015 02:51 PM, Jacob Wisor wrote:
>>> Am 01.04.2015 um 12:56 schrieb Jiri Vanek:
>>>> On 04/01/2015 12:34 PM, Jacob Wisor wrote:
>>>>> Hello there!
>>>>>
>>>>> I have come across a few problems while localizing IcedTea-Web for version 1.6.
>>>>> This patch improves running the html-gen.sh script manually. Okay to push?
>>>>>
>>>>
>>>> The changes looks ok. Few nits inline which you may follow.
>>>>
>>>>> Regards,
>>>>> Jacob
>>>>>
>>>>> html-gen.patch
>>>>>
>>>>>
>>>>> diff -r b7b28ec53a8b -r 239bf3e94c02 AUTHORS
>>>>> --- a/AUTHORS
>>>>> +++ b/AUTHORS
>>>>> @@ -10,7 +10,7 @@
>>>>>   Adam Domurad<adomurad at redhat.com>
>>>>>   Lukasz Dracz<ldracz at redhat.com>
>>>>>   Thomas Fitzsimmons<fitzsim at redhat.com>
>>>>> -Michał Górny <mgorny at gentoo.org  >
>>>>> +Michał Górny<mgorny at gentoo.org>
>>>>>   Mark Greenwood<mark at dcs.shef.ac.uk>
>>>>>   Peter Hatina<phatina at redhat.com>
>>>>>   Andrew John Hughes<ahughes at redhat.com>
>>>>> @@ -26,16 +26,16 @@
>>>>>   Thomas Meyer<thomas at m3y3r.de>
>>>>>   Kurt Miller<kurt at intricatesoftware.com>
>>>>>   Saad Mohammad<smohammad at redhat.com>
>>>>> -Martin Olsson<martin at minimum.se>
>>>>> +Martin Olsson<martin at minimum.se>
>>>>>   Fridrich Strba<fridrich.strba at suse.com>
>>>>>   Andrew Su<asu at redhat.com>
>>>>>   Joshua Sumali<jsumali at redhat.com>
>>>>>   Jiri Vanek<jvanek at redhat.com>
>>>>>   Mark Wielaard<mark at klomp.org>
>>>>> -Jacob Wisor<gitne at excite.co.jp>
>>>>> +Jacob Wisor<gitne at icedtea.classpath.org>
>>>>>   Man Lung Wong<mwong at redhat.com>
>>>>>
>>>>>   This project also includes code from the following projects:
>>>>>
>>>>>   OpenJDK<http://openjdk.java.net/>
>>>>> -Netx<http://jnlp.sourceforge.net/netx/>
>>>>> +NetX<http://jnlp.sourceforge.net/netx/>
>>>>> diff -r b7b28ec53a8b -r 239bf3e94c02 ChangeLog
>>>>> --- a/ChangeLog
>>>>> +++ b/ChangeLog
>>>>> @@ -1,3 +1,14 @@
>>>>> +2015-04-01  Jacob Wisor<gitne at icedtea.classpath.org>
>>>>> +
>>>>> +    * AUTHORS: Fix e-mail address formatting and update my e-mail address
>>>>> +    * ChangeLog: Update my e-mail address to reflect that in AUTHORS
>>>>> +    * Makefile.am: (stamps/netx-html-gen.stamp) Move creating the html-gen
>>>>
>>>> Can you actually be contacted in this address?
>>>
>>> Sure! :-)
>>>
>>>> Isnt more simple or even better to change the address in Authors?
>>>
>>> No, better make it consistent throughout the sources.
>>
>> Yes. Once you are reachable here, then its clear.
>>>
>>>>> +    directory and copying of AUTHORS, COPYING, ChangeLog, and NEWS files into
>>
>>>>> +# Directory where generated HTML files are put out
>>>>> +HTML_GEN_DIR="html-gen"
>>>>> +if [ ! -d "$HTML_GEN_DIR" ]; then
>>>>> +    mkdir -p "$HTML_GEN_DIR"
>>>> Maybe die somewhere here if creation fails?
>>>>
>>>>> +fi
>>>>> +cp -pf AUTHORS COPYING ChangeLog NEWS "$HTML_GEN_DIR"
>>>> And same here, if source files (Authors, News...) do not exists>
>>>
>>> Hmm, I don't think we should bother, it is no different from any of the sed,
>>> grep, or what ever stuff that may fail in the script. On errors, the script
>>> keeps going anyway and presumably returns an error level set by the last
>>> sub-process (which may be 0, indicating a success). So, if we start adding
>>> error level checking here we may as well start adding it everywhere. But, if
>>> you intend to protect the source files in the project root directory it would
>>> suffice to return from the script when the attempt to change directory into
>>> the html-gen directory fails. Just as a reminder; the source files have not
>>> been protected in the current version (tip) of the script either.
>>>>> +cd "$HTML_GEN_DIR"
>>
>>
>> My small fear was that now you can run it in top directory, and if it fails on
>> HTML_GEN_DIR work, then it can actually override sources. Before you had to
>> create dir manually, and so you were pretty sure.
>
> No, you weren't! Please read more carefully what I write. If you did not create the directory before
> hand you would end up with the sources in the root directory "converted" into these pseudo HTML
> files. That's the whole point of this patch. Nobody takes a look into a script before running. You
> just assume it to be safe. Well, that's what I did and boom: the source files had been moved to
> /dev/null. To be fair, there were errors but it has been too late by then. Luckily, we have
> Mercurial to revert mistakes.


Well, yes :)

Please push!

And sorry for delay.


J.
>
>> But maybe I'm just  paranoid.
>
> It's too late for this now.
>
>>
>> The return value will work wrongly in same way, as it did  before with the "\"
>> terminators in makefile. So will not :)
>>
>>>>>
>>>>>   print_debug "Generating HTML content for javaws -about for $REPO_URL.
>>>>> $CHANGESETS changesets, $NEWS_ITEMS news items"
>>>>>   print_debug "Starting sed substitutions"