Apply 2-character indents to tooltool manifests

RESOLVED FIXED in Firefox 55

Status

enhancement
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: rillian, Assigned: rillian)

Tracking

unspecified
mozilla55

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

Our tooltool manifests are written with newlines but without indentation, which makes them harder to read. Modern code editors should be able to maintain json indentation, so I don't think there's a reason to keep them as-is.
This also smooths bumping the rust toolchains; currently manifests aren't updated except for the Android sdk 'versions' array. It would be nice to not have to special-case those. Our in-tree copy of tooltool was updated to produce indented manifests in bug 1325225.
Assignee: nobody → giles
Missed browser/config/tooltool-manifests/linux64/clang.manifest.centos6
Comment on attachment 8846770 [details]
Bug 1346897 - Indent tooltool manifests.

https://reviewboard.mozilla.org/r/119774/#review122146

I also see a few others by grepping for '^"digest":'. Most are b2g related, but ./js/src/devtools/rootAnalysis/build/gcc.manifest and ./testing/config/tooltool-manifests/androidarm_6_0/mach-emulator.manifest stick out. (I'm not sure if those are actually used anywhere).
Attachment #8846770 - Flags: review?(mshal) → review+
I have some patches I was fiddling with locally that add better support to tooltool for reading/writing manifests, so you could modify and round trip them (since I've had such a pain doing this in the past). I think I let those patches fall by the wayside because I'm hoping glandium will fix bug 1313111 soon...
rillian asked for clarification in IRC, and I realize the relevance may not have been clear! I have a patch here that gives tooltool.py the ability to read and write manifests without extraneous diffs, and then I have another patch in which I ran that on every manifest in the tree, and it does produce a bunch of diffs, mostly because of the lack of indentation of our in-tree manifests.

This is what that diff looks like:
https://hg.mozilla.org/users/tmielczarek_mozilla.com/mc/rev/415252474c01
Ok, that sounds like the same inconsistent-indent problem I was trying to fix. I suppose while people are hand-editing these files we either need a lint to maintain the formatting or a merge tool that maintains the indentation it finds.
(In reply to Michael Shal [:mshal] from comment #5)

> I also see a few others by grepping for '^"digest":'. Most are b2g related,
> but ./js/src/devtools/rootAnalysis/build/gcc.manifest and
> ./testing/config/tooltool-manifests/androidarm_6_0/mach-emulator.manifest
> stick out. (I'm not sure if those are actually used anywhere).

Ah, thanks. I missed those. I'll update them as well and rebase around conflicting changes.
Pushed by rgiles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/faf87c0a0f98
Indent tooltool manifests. r=mshal
https://hg.mozilla.org/mozilla-central/rev/faf87c0a0f98
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.