Make generate_buildbot_json.py autoformat .pyl files |
||
Issue descriptionThe script currently uses the ast python package to parse the .pyl files and warn the user if they are unsorted. The ast package gives us line number information, so theoretically the script could actually rewrite the files to be correctly formatted.
,
Sep 19
yapf is useful, and we probably want to use it. It doesn't sort keys though, which is part of why the code I wrote is useful. Unless yapf has some hidden sorting capabilities I don't know of.
,
Sep 19
Good point, yapf won't do that. I would expect ast to lose any comments that the file might have. I'm not aware of any tool that will do what you want. I actually have a side/20% project that probably would do it, but I really doubt we'd want to depend on that given that I don't have the time to maintain it.
,
Sep 19
I would also love something like this for all of our newfound infra configs, so if your side/20% project can handle those, too, perhaps it'd be worth us (not necessarily you specifically) investing a little time into it?
,
Sep 19
I had thought it would work like this:
The objects ast gives us have a lineno attribute. I assume that refers to the first line that the object is defined on. So, the plan is to run the existing logic, which finds out which keys are in the wrong place. Then, you map where each missing key is to the lines which it corresponds to, and swap around lines. Example:
1 {
2 # Some comment here
3 'foo': {
4 ...
5 ...
6 },
7
8 # Some comment here
9 'aa': {
10 ...
11 },
12 }
When we do the ast parsing, we get back an object for 'foo' and 'aa'. They both have line numbers. The lines that key encompass are everything from 'foo' to the next key; 2-8 inclusive. Then you can grab those lines and move them around.
You would mess up the comments, so you might have to do a pass on the actual file and figure out where comments live.
This is pretty specific to how we're using the .pyl files; I don't think it'd work in the general case, but it might work for us.
,
Sep 19
Happy to use your 20% project though, if it works well for this.
,
Sep 19
The 20% thing is a parser generator / code formatter thing that in theory could be made to work for this (it's explicitly intended to be able to handle this sort of thing), but I haven't actually done this for this format as a proof of concept (I've thought about it, but haven't done it). That said, it's in a state that I don't think either of us would want to depend on at this point. We can talk about whether it'd make sense to invest in things here, but this is low enough priority that I doubt it's worth it, and you're probably better off hand-coding something. As far as all of the other config files goes, that's a real problem, but I think the foundation thinking for this is that we want to move to a meta-config file based on skylark (aka python) that would then generate the textproto files. It looks like there is a formatter as part of the Bazel projects.
,
Sep 19
This is the Bazel formatter I found. I have no hands-on experience with it. https://github.com/bazelbuild/buildtools/blob/master/buildifier/README.md
,
Sep 20
FYI I'm going to be uploading CLs to enforce sorting which will link to this bug.
,
Sep 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7eb8b6179d4b482e2e4076e42f2a75dd9c074c97 commit 7eb8b6179d4b482e2e4076e42f2a75dd9c074c97 Author: Stephen Martinis <martiniss@chromium.org> Date: Fri Sep 21 00:17:50 2018 generate_buildbot_json.py: Better handle printed lines This CL changes the tests for generate_buildbot_json.py to capture all printed lines. It requires tests to assert about the contents of these printed lines, and then clear them. It also adds a --verbose flag to the main script, which is passed when checking the configs. Also fixes a test for swarming mixins and how they interact with certain classes of tests. Bug: 886993 Change-Id: I047d74689d72f35bc7d19c8da338c070a8a5d9a5 Reviewed-on: https://chromium-review.googlesource.com/1237239 Commit-Queue: Stephen Martinis <martiniss@chromium.org> Reviewed-by: Kenneth Russell <kbr@chromium.org> Cr-Commit-Position: refs/heads/master@{#593015} [modify] https://crrev.com/7eb8b6179d4b482e2e4076e42f2a75dd9c074c97/testing/buildbot/PRESUBMIT.py [modify] https://crrev.com/7eb8b6179d4b482e2e4076e42f2a75dd9c074c97/testing/buildbot/generate_buildbot_json.py [modify] https://crrev.com/7eb8b6179d4b482e2e4076e42f2a75dd9c074c97/testing/buildbot/generate_buildbot_json_unittest.py |
||
►
Sign in to add a comment |
||
Comment 1 by dpranke@chromium.org
, Sep 19