914935 - pymake doesn't handle colons gracefully in includedep files

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

pymake considers that a colon followed by a slash or backslash is not to be treated as the usual target/dependencies separator. But it doesn't do that for includedep files.
But these files contain full windows paths, including the drive letter, which means the drive letter is considered a target, and everything that follows, dependencies.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Fix pymake handling of colon in includedep files

Comment 2

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/895784bef7fd

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 802718 [details] [diff] [review]
Fix pymake handling of colon in includedep files

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 852249
User impact if declined: Windows builds may fail after some classes of changes because dependencies are not fully defined.
Testing completed (on m-c, etc.): Landed on m-i today. Tested locally by avih.
Risk to taking this patch (and alternatives if risky): Should be low risk
String or IDL/UUID changes made by this patch: None

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/895784bef7fd

Comment 6

[Alex Keybl [:akeybl]]

(In reply to Mike Hommey [:glandium] from comment #3)
> User impact if declined: Windows builds may fail after some classes of
> changes because dependencies are not fully defined.

This has been around for a while - isn't the risk of this breaking something in the 25 timeframe very unlikely since it'll already be frozen? And wouldn't we know if it broke?

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 802718 [details] [diff] [review]
Fix pymake handling of colon in includedep files

26 is on Aurora now and from Alex's comment it looks unlikely this will get uplift approval for Beta but if there's a significant reward please re-nominate for approval-beta