1079893 - Better formatting for Maintenance wiki reconfig updates

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Ed Morley [:emorley]]

Compare:

https://wiki.mozilla.org/ReleaseEngineering/Maintenance#Reconfigs_.2F_Deployments
"bug 1035226 - Only use RUNTIME iff Mule"
and
"bug 1035226 - Pass RUNTIME to Gij for Mule"

Against:
"Bug 1035226 - Only use RUNTIME iff Mulet" (https://hg.mozilla.org/build/mozharness/rev/97f6c98a5437)
and
"Bug 1035226 - Pass RUNTIME to Gij for Mulet, r=ochameau" (https://hg.mozilla.org/build/mozharness/rev/5c900c15ff70)

Need to tweak:

https://hg.mozilla.org/build/tools/file/default/buildfarm/maintenance/end_to_end_reconfig.sh#l341
   341     grep summary "${RECONFIG_DIR}"/*_preview_changes.txt | \
   342         awk '{sub (/ r=.*$/,"");print substr($0, index($0,$2))}' | \
   343         sed 's/[Bb]ug \([0-9]*\):* *-* */\* {{bug|\1}} - /' | \
   344         sed 's/^[ \t]*//;s/[ \t,;]*$//' | \
   345         sed 's/^\([^\*]\)/\* \1/' | \
   346         sort -u >> "${RECONFIG_DIR}/reconfig_update_for_maintenance.wiki"

Not all versions of sed support using escape sequences such as '\t' - you need to insert a literal tab into the script, if we want to protect against them (though I wonder if Hg strips tabs out anyway?).

As such, |sed 's/^[ \t]*//;s/[ \t,;]*$//'| actually matches against the "t" in mulet lol.

Comment 1

[Ed Morley [:emorley]]

http://azaleasays.com/2014/03/07/os-x-sed-does-not-recognize-tab-character/

Comment 2

[Chris Cooper [:coop] (he/him)]

Attachment: Formatting improvements for Maintenance wiki updates

While :pmoore is working on automating bugzilla updates, I figure I'd fix this issue and also make some other formatting changes:

* formatting is now handled by a separate python script
* commit are grouped and sorted by repo
* there are now links to each revision
* there is a global link to a buglist of all the bugs affected by the reconfig. This makes it easier in the interim to update those bugs in bugzilla.

Comment 3

[Pete Moore [:pmoore][:pete]]

(In reply to Ed Morley [:edmorley] from comment #0)
> Compare:
> 
> https://wiki.mozilla.org/ReleaseEngineering/Maintenance#Reconfigs_.
> 2F_Deployments
> "bug 1035226 - Only use RUNTIME iff Mule"
> and
> "bug 1035226 - Pass RUNTIME to Gij for Mule"
> 
> Against:
> "Bug 1035226 - Only use RUNTIME iff Mulet"
> (https://hg.mozilla.org/build/mozharness/rev/97f6c98a5437)
> and
> "Bug 1035226 - Pass RUNTIME to Gij for Mulet, r=ochameau"
> (https://hg.mozilla.org/build/mozharness/rev/5c900c15ff70)
> 
> Need to tweak:
> 
> https://hg.mozilla.org/build/tools/file/default/buildfarm/maintenance/
> end_to_end_reconfig.sh#l341
>    341     grep summary "${RECONFIG_DIR}"/*_preview_changes.txt | \
>    342         awk '{sub (/ r=.*$/,"");print substr($0, index($0,$2))}' | \
>    343         sed 's/[Bb]ug \([0-9]*\):* *-* */\* {{bug|\1}} - /' | \
>    344         sed 's/^[ \t]*//;s/[ \t,;]*$//' | \
>    345         sed 's/^\([^\*]\)/\* \1/' | \
>    346         sort -u >>
> "${RECONFIG_DIR}/reconfig_update_for_maintenance.wiki"
> 
> Not all versions of sed support using escape sequences such as '\t' - you
> need to insert a literal tab into the script, if we want to protect against
> them (though I wonder if Hg strips tabs out anyway?).
> 
> As such, |sed 's/^[ \t]*//;s/[ \t,;]*$//'| actually matches against the "t"
> in mulet lol.

Hey Ed,

Thanks so much for digging into the heart of the matter! Excellent diagnosis, above and beyond the call of duty! Awesome stuff.

Pete

Comment 4

[Ed Morley [:emorley]]

No problem :-)
(I like random bugs like these, reminded me slightly of http://mdzlog.alcor.net/2009/08/15/bohrbugs-openoffice-org-wont-print-on-tuesdays/)

Comment 5

[Pete Moore [:pmoore][:pete]]

Comment on attachment 8504813 [details] [diff] [review]
Formatting improvements for Maintenance wiki updates

Review of attachment 8504813 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for working on this coop, this will be a real benefit. I think it is fine to r+ since it works, but there are a couple of things I'm not sure about:

1) Creating variables for something which cannot have any other value than the default value - my tendency is not to use a variable name in this case as it makes code reading a little trickier, and also means the reader needs to check through the code to see if it changes anywhere
2) I think in general it is nicer if command line utilities output to standard out, and let the calling script decide if it wants to park the output in a file or not - that way you can call the command line utility from the command line and not be concerned with where it is placing its output, and for the calling script, it is always clear where it is putting the output, and you don't need to dig into the utility you are calling to see where it puts it, when looking at the calling script. It also allows easier parallelisation etc, because you don't need to worry about two instances running at the same time and overwriting the same file. So I'd prefer for the python script to write to standard out, and the bash script to redirect that to the file. This also then avoids all the fancy code to decide which directory to write to, working out if it is a relative or absolute path, etc - all that can be avoided (e.g. lines 83 to 87, and in a couple of other places in the script that deal with "outputfile" variable).

::: buildfarm/maintenance/end_to_end_reconfig.sh
@@ +152,5 @@
>  FORCE_RECONFIG="${FORCE_RECONFIG:-0}"
>  MERGE_TO_PRODUCTION="${MERGE_TO_PRODUCTION:-1}"
>  UPDATE_WIKI="${UPDATE_WIKI:-1}"
>  RECONFIG_DIR="${RECONFIG_DIR:-/tmp/reconfig}"
> +RECONFIG_UPDATE_FILE="${RECONFIG_UPDATE_FILE:-reconfig_update_for_maintenance.wiki}"

The other env vars have a default value because they can be set as command line options, so I don't think you need a default here - you could just explicitly set it. I'm also ok with not having a variable for it, since it can't be changed (or i can't think of a reason we would want to change it, or pass in a different value). E.g. we don't set a variable for pending_changes file either, since it is always called pending_changes. It does no harm, but I don't see any gain.

::: buildfarm/maintenance/format_wiki_update.py
@@ +83,5 @@
> +    if outputfile == DEFAULT_OUTPUT_FILE:
> +        outputfile = os.path.join(logdir, DEFAULT_OUTPUT_FILE)
> +    else:
> +        if not os.path.isdir(os.path.dirname(outputfile)):
> +            os.mkdir(os.path.dirname(outputfile))

When I specified a filename that wasn't the default, without a path, this crashed with:

pmoore@Elisandra:~/git/tools/buildfarm/maintenance master $ python format_wiki_update.py --logdir /tmp/reconfig --output-file reconfig_update_for_maintenance.wiki2
Traceback (most recent call last):
  File "format_wiki_update.py", line 134, in <module>
    write_wiki_output(args.logdir, args.outputfile)
  File "format_wiki_update.py", line 87, in write_wiki_output
    os.mkdir(os.path.dirname(outputfile))
OSError: [Errno 2] No such file or directory: ''

Comment 6

[Chris Cooper [:coop] (he/him)]

Comment on attachment 8504813 [details] [diff] [review]
Formatting improvements for Maintenance wiki updates

Review of attachment 8504813 [details] [diff] [review]:
-----------------------------------------------------------------

Landed with Pete's nits addressed. The script prints the output STDOUT now, rather than to a file, and lets the caller (end_to_end_reconfig.sh) handle the output as it chooses.

https://hg.mozilla.org/build/tools/rev/220b0fe55344

Comment 7

[Pete Moore [:pmoore][:pete]]

I think some gremlins might have slipped in:

https://hg.mozilla.org/build/tools/rev/220b0fe55344#l1.51
https://hg.mozilla.org/build/tools/rev/220b0fe55344#l1.63