581516 - Revisions that start with 0 (with no hex chars) shouldn't be treated as an octal value for SourceStamp

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Chris AtLee [:catlee]]

We did builds for revision 023554677044288a5b925a7b23dc2adbd607a761, short revision 023554677044.  Once these builds got to talos, they reported themselves as revision 2645786148, which is what you get if you treat 023554677044 as an octal value.

Talos grabs the revision to report to the graph server from application.ini.

Comment 1

[(not currently active) Ted Mielczarek]

I'm fairly certain this is a Preprocessor.py bug, and probably fallout from bug 569373.

Comment 2

[(not currently active) Ted Mielczarek]

Actually, after further review, I think it's just failure to properly quote values, and now exposed by that bug.

Comment 3

[(not currently active) Ted Mielczarek]

Attachment: Strip quotes from cmdline -D values in preprocessor.py

This + escaping the quotes in browser/app/Makefile.in fixes the problem, but I'm not sure if this will break anything else.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Revision 036464778311 died during build today on tinderbox.

Comment 6

[(not currently active) Ted Mielczarek]

catlee's alternate proposal was just to make Preprocessor.py not try to handle octal literals, since it's likely that nobody cares about them anyway. I did that to appease Pike in bug 569373 since expressions were partially parsed that way. We could just strip out all the octal-handling code and be done with it.

Comment 7

[Chris AtLee [:catlee]]

(In reply to comment #6)
> catlee's alternate proposal was just to make Preprocessor.py not try to handle
> octal literals, since it's likely that nobody cares about them anyway. I did
> that to appease Pike in bug 569373 since expressions were partially parsed that
> way. We could just strip out all the octal-handling code and be done with it.

+1

Comment 8

[(not currently active) Ted Mielczarek]

Attachment: Stop handling octal literals in Preprocessor.py

Here's a patch that implements that. The added unit tests are mostly unrelated, but I wrote them when verifying the handling of commandline arguments with quotes, and I think they're good to have just to make it clear what our current behavior is. We have existing tests for octal literals that I wrote on the previous bug, and they all still pass (the octal literals just get parsed as decimal, but the tests show that we do so consistently everywhere).

Comment 9

[(not currently active) Ted Mielczarek]

Comment on attachment 460507 [details] [diff] [review]
Stop handling octal literals in Preprocessor.py

Simple build bustage fix for unlucky changeset IDs. The behavior here is covered by existing unit tests.

Comment 10

[(not currently active) Ted Mielczarek]

Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/105461023211