Patchwork D1167: makefile: add target to apply clang-format in-place

login
register
mail settings
Submitter phabricator
Date Oct. 17, 2017, 9:44 p.m.
Message ID <differential-rev-PHID-DREV-buvzeicv23ajvscvubil-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/25146/
State Superseded
Headers show

Comments

phabricator - Oct. 17, 2017, 9:44 p.m.
durin42 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This makes it easy to reformat files after you finish editing them.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1167

AFFECTED FILES
  Makefile

CHANGE DETAILS




To: durin42, #hg-reviewers
Cc: mercurial-devel
phabricator - Oct. 26, 2017, 3:42 a.m.
av6 added a comment.


  I'm a bit late to the party, but wanted to remind people that targets that are not actual files (in this case, a file named "format-c") should also go into .PHONY list. Nothing bad will happen right now (unless such file appears at some point), but adding it to "phony" targets helps keeping makefile in proper shape. (Looking at the already unwieldy .PHONY list I wish Make had another way to mark targets "phony").

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1167

To: durin42, #hg-reviewers, indygreg
Cc: av6, mercurial-devel
phabricator - Oct. 26, 2017, 3:47 a.m.
durin42 added a comment.


  d'oh - could you send a patch to fix that?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1167

To: durin42, #hg-reviewers, indygreg
Cc: av6, mercurial-devel
phabricator - Oct. 26, 2017, 4:01 a.m.
av6 added a comment.


  Yep, mailed.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1167

To: durin42, #hg-reviewers, indygreg
Cc: av6, mercurial-devel
Matt Harbison - Oct. 27, 2017, 1:14 a.m.
On Wed, 25 Oct 2017 23:42:04 -0400, av6 (Anton Shestakov)  
<phabricator@mercurial-scm.org> wrote:

> av6 added a comment.
>
>
>   I'm a bit late to the party, but wanted to remind people that targets  
> that are not actual files (in this case, a file named "format-c") should  
> also go into .PHONY list. Nothing bad will happen right now (unless such  
> file appears at some point), but adding it to "phony" targets helps  
> keeping makefile in proper shape. (Looking at the already unwieldy  
> .PHONY list I wish Make had another way to mark targets "phony").

Split up the .PHONY declaration and put it in front of each target, so  
it's more obvious when one is missing?

https://unix.stackexchange.com/questions/217295/phony-all-rules-in-gnu-make-file
Anton Shestakov - Oct. 27, 2017, 1:36 p.m.
On Thu, 26 Oct 2017 21:14:27 -0400
"Matt Harbison" <mharbison72@gmail.com> wrote:

> On Wed, 25 Oct 2017 23:42:04 -0400, av6 (Anton Shestakov)  
> <phabricator@mercurial-scm.org> wrote:
> 
> > av6 added a comment.
> >
> >
> >   I'm a bit late to the party, but wanted to remind people that targets  
> > that are not actual files (in this case, a file named "format-c") should  
> > also go into .PHONY list. Nothing bad will happen right now (unless such  
> > file appears at some point), but adding it to "phony" targets helps  
> > keeping makefile in proper shape. (Looking at the already unwieldy  
> > .PHONY list I wish Make had another way to mark targets "phony").  
> 
> Split up the .PHONY declaration and put it in front of each target, so  
> it's more obvious when one is missing?
> 
> https://unix.stackexchange.com/questions/217295/phony-all-rules-in-gnu-make-file

It's not really fit for stable, being a refactoring and all, but I did
this: https://bitbucket.org/av6/hg-wip/commits/4c710586c899b8c

Looks better, maybe? What about non-GNU make alternatives, does BSD
make support this, do we care?

It does indeed make phony targets missing a declaration a lot more
obvious, for example 3 wheels-related ones, I think they also are
actually phony.
Matt Harbison - Oct. 28, 2017, 5:36 a.m.
> On Oct 27, 2017, at 9:36 AM, Anton Shestakov <av6@dwimlabs.net> wrote:
> 
> On Thu, 26 Oct 2017 21:14:27 -0400
> "Matt Harbison" <mharbison72@gmail.com> wrote:
> 
>> On Wed, 25 Oct 2017 23:42:04 -0400, av6 (Anton Shestakov)  
>> <phabricator@mercurial-scm.org> wrote:
>> 
>>> av6 added a comment.
>>> 
>>> 
>>>  I'm a bit late to the party, but wanted to remind people that targets  
>>> that are not actual files (in this case, a file named "format-c") should  
>>> also go into .PHONY list. Nothing bad will happen right now (unless such  
>>> file appears at some point), but adding it to "phony" targets helps  
>>> keeping makefile in proper shape. (Looking at the already unwieldy  
>>> .PHONY list I wish Make had another way to mark targets "phony").  
>> 
>> Split up the .PHONY declaration and put it in front of each target, so  
>> it's more obvious when one is missing?
>> 
>> https://unix.stackexchange.com/questions/217295/phony-all-rules-in-gnu-make-file
> 
> It's not really fit for stable, being a refactoring and all, but I did
> this: https://bitbucket.org/av6/hg-wip/commits/4c710586c899b8c
> 
> Looks better, maybe? What about non-GNU make alternatives, does BSD
> make support this, do we care?

Not sure about non-GNU.  I thought that when I did builds for FreeBSD, I had to use gmake.  And a coworker ran through the build on Illumos this week, and commented that GNU tools were needed.

> It does indeed make phony targets missing a declaration a lot more
> obvious, for example 3 wheels-related ones, I think they also are
> actually phony.
Augie Fackler - Oct. 30, 2017, 1:28 a.m.
> On Oct 27, 2017, at 9:36 AM, Anton Shestakov <av6@dwimlabs.net> wrote:
> 
> On Thu, 26 Oct 2017 21:14:27 -0400
> "Matt Harbison" <mharbison72@gmail.com> wrote:
> 
>> On Wed, 25 Oct 2017 23:42:04 -0400, av6 (Anton Shestakov)  
>> <phabricator@mercurial-scm.org> wrote:
>> 
>>> av6 added a comment.
>>> 
>>> 
>>>  I'm a bit late to the party, but wanted to remind people that targets  
>>> that are not actual files (in this case, a file named "format-c") should  
>>> also go into .PHONY list. Nothing bad will happen right now (unless such  
>>> file appears at some point), but adding it to "phony" targets helps  
>>> keeping makefile in proper shape. (Looking at the already unwieldy  
>>> .PHONY list I wish Make had another way to mark targets "phony").  
>> 
>> Split up the .PHONY declaration and put it in front of each target, so  
>> it's more obvious when one is missing?
>> 
>> https://unix.stackexchange.com/questions/217295/phony-all-rules-in-gnu-make-file
> 
> It's not really fit for stable, being a refactoring and all, but I did
> this: https://bitbucket.org/av6/hg-wip/commits/4c710586c899b8c
> 
> Looks better, maybe? What about non-GNU make alternatives, does BSD
> make support this, do we care?

Our makefile already doesn’t work with BSD Make. I’ve tried to make it work in the past, but we’re doing too many clever things.

> 
> It does indeed make phony targets missing a declaration a lot more
> obvious, for example 3 wheels-related ones, I think they also are
> actually phony.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -122,6 +122,10 @@ 
 check-code:
 	hg manifest | xargs python contrib/check-code.py
 
+format-c:
+	clang-format --style file -i \
+	  `hg files 'set:(**.c or **.h) and not "listfile:contrib/clang-format-blacklist"'`
+
 update-pot: i18n/hg.pot
 
 i18n/hg.pot: $(PYFILES) $(DOCFILES) i18n/posplit i18n/hggettext