965120 - clang-format should not depend on having a mercurial repository

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Bobby Holley (:bholley)]

I use git. When I do |mach clang-format|, I get the following:

bholley@extraordinary /files/mozilla/repos/main (master) $ mach clang-format
Download clang-format executables from https://people.mozilla.org/~ajones/clang-format/ (yN)? y
Downloading https://people.mozilla.org/~ajones/clang-format/darwin/clang-format-3.5 to /Users/bholley/.mozbuild/clang-format-3.5
Downloading https://people.mozilla.org/~ajones/clang-format/clang-format-diff-3.5 to /Users/bholley/.mozbuild/clang-format-diff-3.5
abort: no repository found in '/files/mozilla/repos/main' (.hg not found)!

Comment 1

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Add --show option to mach clang-format;

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Use .clang-format to set tree default format;

Comment 3

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Git support for mach clang-format;

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8367118 [details] [diff] [review]
Git support for mach clang-format;

Review of attachment 8367118 [details] [diff] [review]:
-----------------------------------------------------------------

::: tools/mach_commands.py
@@ +355,5 @@
>  
>          from subprocess import Popen, PIPE
> +        diff_process = Popen(["hg", "diff", "-U0", "-r", "tip^",
> +                              "--include", "glob:**.c", "--include", "glob:**.cpp", "--include", "glob:**.h",
> +                              "--exclude", "listfile:.clang-format-ignore"], stdout=PIPE)

I /think/ diff_process will be true-ish even if the process exists with a non-0 exit code.

I think you should key off .hg and .git to determine which binary to run.

Comment 5

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Git support for mach clang-format;

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8367724 [details] [diff] [review]
Git support for mach clang-format;

Review of attachment 8367724 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxPlatform.cpp
@@ +369,5 @@
>          NS_RUNTIMEABORT("Already started???");
>      }
> +    for (int i = 0; i < 10; i++) {
> +      printf("I like swords\n");
> +    }

That's nice. But please don't check this in :)

::: tools/mach_commands.py
@@ +354,5 @@
>              return 1
>  
>          from subprocess import Popen, PIPE
> +
> +        if os.path.exists(".hg"):

This will only work if cwd == topsrcdir. Fix it now or with a follow-up bug.

Comment 7

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Gregory Szorc [:gps] from comment #6)
> > +    for (int i = 0; i < 10; i++) {
> > +      printf("I like swords\n");
> > +    }
> 
> That's nice. But please don't check this in :)

There's me checking in some reformatting test. It turns out that the indentation will be 2 for changed lines but it respects the indent of 4 from above.

> ::: tools/mach_commands.py
> @@ +354,5 @@
> >              return 1
> >  
> >          from subprocess import Popen, PIPE
> > +
> > +        if os.path.exists(".hg"):
> 
> This will only work if cwd == topsrcdir. Fix it now or with a follow-up bug.

Line 342 above is 'os.chdir(self.topsrcdir)' so we should be OK.

Comment 8

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d44de38ad97f
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a77a99c84d5
https://hg.mozilla.org/integration/mozilla-inbound/rev/f99313fc00b5

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d44de38ad97f
https://hg.mozilla.org/mozilla-central/rev/4a77a99c84d5
https://hg.mozilla.org/mozilla-central/rev/f99313fc00b5