Last Comment Bug 740452 - client.mk: add a convenience target for check-sync-dirs so the test can be run independent of configure
: client.mk: add a convenience target for check-sync-dirs so the test can be ru...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Joey Armstrong [:joey]
:
: Gregory Szorc [:gps]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-29 10:12 PDT by Joey Armstrong [:joey]
Modified: 2012-04-06 11:39 PDT (History)
3 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
new target for check-sync-dirs so test can be run independent of configure (1.73 KB, patch)
2012-03-29 10:34 PDT, Joey Armstrong [:joey]
ted: review+
Details | Diff | Splinter Review
move check-sync-dirs test to a named target so it can be run indepen dent of configure (1.66 KB, patch)
2012-04-05 11:35 PDT, Joey Armstrong [:joey]
no flags Details | Diff | Splinter Review

Description Joey Armstrong [:joey] 2012-03-29 10:12:57 PDT

    
Comment 1 Joey Armstrong [:joey] 2012-03-29 10:34:38 PDT
Created attachment 610587 [details] [diff] [review]
new target for check-sync-dirs so test can be run independent of configure

Code consolidation and will be helpful for submission scripts to trap silly errors before they reach try.
Comment 2 Joey Armstrong [:joey] 2012-04-05 10:13:19 PDT
ping on the code review
Comment 3 Ted Mielczarek [:ted.mielczarek] 2012-04-05 11:07:19 PDT
Comment on attachment 610587 [details] [diff] [review]
new target for check-sync-dirs so test can be run independent of configure

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

::: client.mk
@@ +425,5 @@
> +## Sanity check $X and js/src/$X are in sync
> +.PHONY: check-sync-dirs
> +check-sync-dirs: check-sync-dirs-build check-sync-dirs-config
> +check-sync-dirs-%:
> +	@$(PYTHON) $(TOPSRCDIR)/js/src/config/check-sync-dirs.py $(TOPSRCDIR)/js/src/$(subst check-sync-dirs-,$(NULL),$@) $(TOPSRCDIR)/$(subst check-sync-dirs-,$(NULL),$@)

Can't you just use $* here instead of this subst?
Comment 4 Joey Armstrong [:joey] 2012-04-05 11:23:11 PDT
(In reply to Ted Mielczarek [:ted] from comment #3)
> Comment on attachment 610587 [details] [diff] [review]
> new target for check-sync-dirs so test can be run independent of configure
> 
> Review of attachment 610587 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: client.mk
> @@ +425,5 @@
> > +## Sanity check $X and js/src/$X are in sync
> > +.PHONY: check-sync-dirs
> > +check-sync-dirs: check-sync-dirs-build check-sync-dirs-config
> > +check-sync-dirs-%:
> > +	@$(PYTHON) $(TOPSRCDIR)/js/src/config/check-sync-dirs.py $(TOPSRCDIR)/js/src/$(subst check-sync-dirs-,$(NULL),$@) $(TOPSRCDIR)/$(subst check-sync-dirs-,$(NULL),$@)
> 
> Can't you just use $* here instead of this subst?

Nice, yes that would do the trick and will be more efficient.
Comment 5 Joey Armstrong [:joey] 2012-04-05 11:35:39 PDT
Created attachment 612624 [details] [diff] [review]
move check-sync-dirs test to a named target so it can be run indepen dent of configure

same patch as last time with $(subst ) replaced with $* per Ted's suggestion.
r=ted
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-04-05 15:35:37 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7ebb831c329
Comment 7 Matt Brubeck (:mbrubeck) 2012-04-06 11:39:08 PDT
https://hg.mozilla.org/mozilla-central/rev/e7ebb831c329

Note You need to log in before you can comment on or make changes to this bug.