Closed Bug 965120 Opened 10 years ago Closed 10 years ago

clang-format should not depend on having a mercurial repository

Categories

(Firefox Build System :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla29

People

(Reporter: bholley, Assigned: ajones)

Details

Attachments

(3 files, 1 obsolete file)

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)!
Assignee: nobody → ajones
Status: NEW → ASSIGNED
Attachment #8367116 - Flags: review?(gps) → review+
Attachment #8367117 - Flags: review?(gps) → review+
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.
Attachment #8367118 - Flags: review?(gps) → feedback+
Attachment #8367118 - Attachment is obsolete: true
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.
Attachment #8367724 - Flags: review?(gps) → review+
(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.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.