1611581 - Mismatch of clang-format, with/without --show

Status: NEW

(Developer Infrastructure :: Lint and Formatting, defect, P3)

Description

[Kai Engert [:KaiE:]]

mach clang-format --path  comm/mailnews/local/src/nsNoIncomingServer.h

Result: file isn't touched, no changes are applied.

mach clang-format --show --path  comm/mailnews/local/src/nsNoIncomingServer.h

Result: several suggested changes are printed.

How can this difference be explained?

Expected: It shouldn't matter if --show is given or not, both scenarios should suggest the same code formatting.

Comment 1

[Jorg K (CEST = GMT+2)]

Hmm, not for me:

$ mach clang-format --show -p comm/mailnews/local/src/nsNoIncomingServer.h
Processing 1 file(s)...

hg diff shows no change.

Comment 2

[Magnus Melin [:mkmelin]]

Not difference for me either.

Comment 3

[Kai Engert [:KaiE:]]

darktrojan saw the problem, too.

I think I found the cause.

Jörg, Magnus, is my assumption correct, you are either building "in tree", or, if you're using an object directory, that it is "below" your mozilla code directory?

darktrojan, is my assumption correct, that you are using an object directory, which is outside of the source code area? That's what I use.

I found that "clang-format --show" copies the source file into a temporary directory inside the object directory. Then clang-format doesn't find the .clang-format file in any of the parent directories, and uses default formatting rules.

The mach script should copy the .clang-format into the top level of the object directory. I did that manually, then --show gives the expected result.

Comment 4

[Kai Engert [:KaiE:]]

See comment 3.

If the object directory is outside the source tree, e.g.:
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../obj-thunder-opt

then "clang-format --show" will not see the .clang-format rules files inside the source tree.

I suggest that mach clang-format should copy the .clang-format file into the object directory.

Comment 5

[Kai Engert [:KaiE:]]

Sigh. When working with Thunderbird, we have both mozilla/.clang-format and mozilla/comm/.clang-format with different rules. And the temporary copy doesn't use the path.

To respect that, it would be ideal to avoid the temporary copy with --show. Instead, don't use the in-place option. Redirect output to a temporary file, then either move the file into place, or use it to diff.

Comment 6

[Jorg K (CEST = GMT+2)]

Jörg, Magnus, is my assumption correct, you are either building "in tree", or, if you're using an object directory, that it is "below" your mozilla code directory?

Yes, next to comm/

When working with Thunderbird, we have both mozilla/.clang-format and mozilla/comm/.clang-format with different rules.

Yes, because M-C changed their rules half way through the C-C reformatting effort :-( - So instead of reformatting it all again, the "old" rules got copied to Thunderbird. The differences are related to the perennial discussion of where the pointer star should go :-(

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Yes, because M-C changed their rules half way through the C-C reformatting effort :-( -

This isn't exactly that. We were formatting this rule depending on the current file (using the most common approach in the file).
That stuff landed after the big reformat as we didn't notice the issue before.
See bug 1547143

Comment 8

[Jorg K (CEST = GMT+2)]

Hmm, which part of my statement was incorrect or not precise? M-C rules changed in the bug you quoted half way during C-C reformatting. You chose to reformat a small percentage of M-C again, I chose not to reformat the majority of our C-C code again.

Comment 9

[Rob Lemley [:rjl]]

If you don't set an object directory explicitly in your mozconfig, the default is to put it inside your source checkout. Somewhere in the Firefox build howtos is states various reasons why that may not be desirable and suggests setting MOZ_OBJDIR so it's outside your source tree. This is what Taskcluster does*. Since automation seems like a likely goal, making mach clang-format work like this for Thunderbird builds will need to happen first.

I looked real quickly at DTN, and I don't see MOZ_OBJDIR mentioned. Perhaps it should be.

• Within Taskcluster, source checkout directories are supposed to be treated as readonly so that they can be cached between build tasks. If a subsequent task using the same revision finds changes, a new clone is done. There's really nothing stopping a job from writing to the checkout directory once hg robustcheckout is done though.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:ahal, could you have a look please?

For more information, please visit auto_nag documentation.