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

Jacob Wisor gitne at gmx.de
Thu Apr 2 01:33:28 UTC 2015


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.

> 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"


More information about the distro-pkg-dev mailing list