620814 - Add ability to add a description string to the avm shell build, also automatically set the avmshell build code to hg tip

Status: VERIFIED FIXED

(Tamarin Graveyard :: Build Config, defect)

Description

[Chris Peyer]

Additional avmshell information would be useful when running and comparing performance tests.  The two requested improvements are

1. Automatically set the AVMPLUS_BUILD_CODE to the current repos tip
2. Add the ability to add a description string to the avmplus usage page in order to help define what the current avmshell build is.

Comment 1

[Chris Peyer]

Attachment: Add HGVERSION and AVMPLUS_DESC

1. Now defines HGVERSION in the makefile - automatically sets the version to the current tip
2. adds --enable-desc="..." to configure.py, if defined, description is shown below the version info when displaying the avmshell help screen
3. alpha-sort the configure.py help strings

Comment 2

[Chris Peyer]

buildbot note: compile-generic.sh script can remove the call to util-update-version.sh once this lands.

Comment 3

[Felix S. Klock II [:pnkfelix, :fklock]]

Comment on attachment 499179 [details] [diff] [review]
Add HGVERSION and AVMPLUS_DESC

Very Cool.

1. However, it looks like I am expected to only set the AVMPLUS_DESC at the time i invoke configure.py.  I like having that available as an option, but I also want to be able to override that setting when I invoke make itself.

Can we generalize it slightly so that AVMPLUS_DESC is set as its own variable (on its own line) in the generated Makefile?

2. Shouldn't the description be part of the -Dversion output?  Right now it seems like I can only extract it by invoking -Dhelp, which is not ideal if my goal is to emphasis the Description in e.g. a command transcript since in that case I do not want the deluge of usage output.

I really like this though!

Comment 4

[Chris Peyer]

Attachment: Address comment 3

1. AVMPLUS_DESC now a Makefile variable
2. Description is output for both -Dhelp and -Dversion (when defined)
3. --enable-shell now set to true by default

Comment 5

[Chris Peyer]

Attachment: incorporate feedback from brent

- Add the ability to specify simple flavored make variables (:= vs. =)
- Switch from using tip revision to current working revision

Comment 6

[Felix S. Klock II [:pnkfelix, :fklock]]

Comment on attachment 515132 [details] [diff] [review]
incorporate feedback from brent

1. "hg id" will produce output like this
  % hg id -n
  5987+5988+
if one's repo is in the midst of a merge; what's going to happen in your code to define HGVERSION in that case?  

2. Could "hg parents" be used to simplify (and possibly correct) that definition of HGVERSION?  I'm thinking something like:
  HGVERSION = "$(shell hg parents --template '{rev}:{node|short} ')"

R+; I trust you'll resolve this in some sane manner.

----

Aside: I wrote a screed about the term "recursively expanded", but then realized that you're just reusing GNU make's silly terminology (its silly b/c both kinds of variables get expanded via a recursive process; the issue is about eagerness/strictness i.e. evaluation order).  Screed deleted, or at least neutered into that parenthetical note.

Comment 7

[Chris Peyer]

Attachment: updated patch based on fklockii feedback

- Switched to using hg parents
- Note that HGVERSION uses recursive=False (aka var set with ":=") otherwise make repeats the shell call a few times slowing make down.

Comment 8

[Tamarin Bot]

changeset: 5988:2831e40981d9
user:      Chris Peyer <[e-mail]>
summary:   Bug 620814: Automatically detect hg revision and set the build string; Add ability to give the avmshell a description (via --enable-desc= or using the AVMPLUS_DESC make var);  Set --enable-shell to True by default. (r=fklockii)

http://hg.mozilla.org/tamarin-redux/rev/2831e40981d9

Comment 9

[Tamarin Bot]

changeset: 5990:f6e661ea6dbe
user:      Chris Peyer <[e-mail]>
summary:   Bug 620814: Remove AVMPLUS_DESC from windows makefiles due to cygwin-wrapper.sf not passing AVMPLUS_DESC strings properly to cl.exe (r=cpeyer)

http://hg.mozilla.org/tamarin-redux/rev/f6e661ea6dbe