Closed Bug 1346897 Opened 4 years ago Closed 4 years ago

Apply 2-character indents to tooltool manifests


(Firefox Build System :: General, enhancement)

Not set


(firefox55 fixed)

Tracking Status
firefox55 --- fixed


(Reporter: rillian, Assigned: rillian)



(1 file)

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.

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 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:
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
Indent tooltool manifests. r=mshal
Closed: 4 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.