Closed Bug 1043390 Opened 6 years ago Closed 6 years ago

The usage of the git command in rcs.mk causes build warnings on Windows where git is not part of mozilla-build

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla34

People

(Reporter: ehsan, Assigned: ehsan)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

No description provided.
Assignee: nobody → ehsan
Attachment #8461551 - Flags: review?(gps)
Comment on attachment 8461551 [details] [diff] [review]
Stop using the git command in rcs.mk since mozilla-build doesn't ship git

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

::: config/makefiles/rcs.mk
@@ +44,2 @@
>  getSourceRepo = \
> +  $(shell cd $(topsrcdir) && cat .git/`cat .git/HEAD | cut -d ':' -d ' ' -f 2`)

This assumes many things that can be wrong. First, that HEAD is a symbolic ref. Second, that the corresponding ref is not packed.
Attachment #8461551 - Flags: review?(gps) → review-
If you don't have git, how do you end up with a .git directory in your mozilla-central source and no .hg directory?
(In reply to Mike Hommey [:glandium] from comment #3)
> If you don't have git, how do you end up with a .git directory in your
> mozilla-central source and no .hg directory?

You are running Git from a different msys environment, you are using TortoiseGit, etc.
I'd rather remove the whole getSourceRepo thing for local builds. It's pretty much pointless.
OK, fine.  FWIW right now messages about git not being found spam every single top-level build on Windows for git users.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Actually the issue still remains.  Please suggest another approach for fixing it.  Thanks!
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Summary: Stop using the git command in rcs.mk since mozilla-build doesn't ship git → The usage of the git command in rcs.mk causes build warnings on Windows where git is not part of mozilla-build
Assignee: ehsan → nobody
(In reply to Mike Hommey [:glandium] from comment #5)
> I'd rather remove the whole getSourceRepo thing for local builds. It's
> pretty much pointless.

This is used for telemetry now: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/Makefile.in#11

Taras, is it ok to disable this somehow for local builds?
Flags: needinfo?(taras.mozilla)
Yeah,
It seems reasonable to predicate this on MOZILLA_OFFICIAL.
Flags: needinfo?(taras.mozilla)
Assignee: nobody → ehsan
Attachment #8469567 - Flags: review?(mh+mozilla)
Comment on attachment 8469567 [details] [diff] [review]
Don't use getSourceRepo on local builds; r=glandium

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

::: build/Makefile.in
@@ +37,1 @@
>  source_repo ?= $(call getSourceRepo,$(topsrcdir)/$(MOZ_BUILD_APP)/..)

Note that instead of wrapping all calls to getSourceRepo, you could make getSourceRepo itself do nothing if MOZILLA_OFFICIAL is not defined.

::: toolkit/mozapps/installer/packager.mk
@@ +199,5 @@
>  SPEC_FILE = $(RPMBUILD_SPECDIR)/mozilla.spec
>  RPM_INCIDENTALS=$(topsrcdir)/toolkit/mozapps/installer/linux/rpm
>  
> +ifdef MOZ_SOURCE_REPO
> +SOURCE_REPO_RPM_CMD = --define 'moz_source_repo $(MOZ_SOURCE_REPO)'

You could move that inline with $(if $(MOZ_SOURCE_REPO),--define ... )

@@ +773,5 @@
>  make-buildinfo-file:
>  	$(PYTHON) $(MOZILLA_DIR)/toolkit/mozapps/installer/informulate.py \
>  		$(MOZ_BUILDINFO_FILE) \
>  		BUILDID=$(BUILDID) \
> +		$(SOURCE_REPO_BUILDINFO) \

$(addprefix MOZ_SOURCE_REPO=,$(MOZ_SOURCE_REPO))
Attachment #8469567 - Flags: review?(mh+mozilla) → review+
We need to mark this test as failing in unofficial builds:

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f37cf9a01fd
https://hg.mozilla.org/mozilla-central/rev/0f37cf9a01fd
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You pe0ople really love to screw up my build process don't you.
That comment had to do with landing the patch in comment 11.
Can you please file a new bug with more specific information on what broke and CC me?
Really just me venting.  I have depended on this for some time for this to work, but I just updated within the last week my builds webpage to document that this works:

"These builds are essentially the same as the corresponding Official mozilla-central Nightly Builds (about:buildconfig will reveal the corresponding changeset)"

and within a week after documenting that it breaks.  Seems like an odd coincidence, and as Gibbs says "I don't believe in coincidences!" ;-)
I fixed it in my builds by adding the following to my build patch:

diff --git a/toolkit/content/buildconfig.html b/toolkit/content/buildconfig.html
--- a/toolkit/content/buildconfig.html
+++ b/toolkit/content/buildconfig.html
@@ -17,16 +17,17 @@
   </style>
 </head>
 <body class="aboutPageWideContainer">
 <h1>about:buildconfig</h1>
 #ifdef BUILD_HOSTNAME
 <h2>Build Machine</h2>
 <p>@BUILD_HOSTNAME@</p>
 #endif
+#define SOURCE_REPO https://hg.mozilla.org/mozilla-central
 #ifdef SOURCE_REPO
 #ifdef SOURCE_CHANGESET
 <h2>Source</h2>
 <p>Built from <a href="@SOURCE_REPO@/rev/@SOURCE_CHANGESET@">@SOURCE_REPO@/rev/@SOURCE_CHANGESET@</a></p>
 #endif
 #elifdef SOURCE_GIT_COMMIT
 <h2>Source</h2>
 <p>Built from git commit <a href="#">@SOURCE_GIT_COMMIT@</a></p>
Oh, I'm sorry about that.  FWIW, doing this was not my idea, but in order to restore the old behavior locally, you can apply this patch to your tree:

diff --git a/toolkit/content/Makefile.in b/toolkit/content/Makefile.in
--- a/toolkit/content/Makefile.in
+++ b/toolkit/content/Makefile.in
@@ -11,21 +11,19 @@ DEFINES += \
   -DCPPFLAGS='$(CPPFLAGS)' \
   $(NULL)

 MOZ_SOURCE_STAMP ?= $(shell hg -R $(topsrcdir) parent --template='{node|short}\n' 2>/dev/null)
 ifdef MOZ_SOURCE_STAMP
 DEFINES += -DSOURCE_CHANGESET='$(MOZ_SOURCE_STAMP)'
 endif

-ifdef MOZILLA_OFFICIAL
 source_repo ?= $(call getSourceRepo)
 ifneq (,$(filter http%,$(source_repo)))
   DEFINES += -DSOURCE_REPO='$(source_repo)'
 else ifneq (,$(strip $(source_repo)))
   DEFINES += -DSOURCE_GIT_COMMIT='$(source_repo)'
 endif
-endif

 ifndef BUILD_HOSTNAME
   BUILD_HOSTNAME = $(shell hostname -s || hostname)
 endif
 DEFINES += -DBUILD_HOSTNAME='$(BUILD_HOSTNAME)'
(In reply to Bill Gianopoulos [:WG9s] from comment #19)
> "These builds are essentially the same as the corresponding Official
> mozilla-central Nightly Builds (about:buildconfig will reveal the
> corresponding changeset)"

Why are you not building with MOZILLA_OFFICIAL?
Well they are not Official builds and I was not sure what else defining that might change.  I really only wanted about:buildconfig to show the changeset which I also had to fake anyway because I was adding more changesets but wanted about:buildconfig to show what the base build that my patches were applied on top of.
Hmm, perhaps we should rename MOZILLA_OFFICIAL to something better.  It enables the things that are enabled on our automation builds mostly.  I think you should be able to use it.
We should probably split it in two, instead of renaming it. There are a couple things I'm not sure should be in a renamed flag (like enabling telemetry and crash submit url)
Oh yes that was another recent change that caused me issues.  I had to start building with --disable-crashreporter because it started submitting crash reports as if they were from an official build.  I had been building with it enabled because back in the day I was the one who helped get crashreporter working on linux x86_64 builds.
Oh and there did used to be and have no clue if it still exists BUILD_OFFICIAL variable, which I do define because way back when I started trying to do nightly type builds it was required..
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.