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

RESOLVED FIXED

Status

defect
--
major
RESOLVED FIXED
9 years ago
2 years ago

People

(Reporter: catlee, Assigned: ted)

Tracking

(Blocks 1 bug)

Trunk
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
I'm fairly certain this is a Preprocessor.py bug, and probably fallout from bug 569373.
Actually, after further review, I think it's just failure to properly quote values, and now exposed by that bug.
This + escaping the quotes in browser/app/Makefile.in fixes the problem, but I'm not sure if this will break anything else.
Revision 036464778311 died during build today on tinderbox.
Summary: Revisions that start with 0 shouldn't be treated as an octal value for SourceStamp → Revisions that start with 0 (with no hex chars) shouldn't be treated as an octal value for SourceStamp
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.
(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
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).
Assignee: nobody → ted.mielczarek
Attachment #459895 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #460507 - Flags: review?(l10n)
Attachment #460507 - Flags: review?(l10n) → review+
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.
Attachment #460507 - Flags: approval2.0?
blocking2.0: --- → betaN+
Attachment #460507 - Flags: approval2.0?
Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/105461023211
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Duplicate of this bug: 593916
Blocks: 593916
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.