Closed Bug 1040009 Opened 9 years ago Closed 8 years ago

.mozconfig configure options are ignored if objdir path is absolute

Categories

(Thunderbird :: Build Config, defect)

Product:
Thunderbird
Component:
Build Config
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(thunderbird34 fixed)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 34.0
Tracking Flags:
Tracking Status
thunderbird34 --- fixed

People

(Reporter: Taraman, Assigned: jcranmer)

References

Details

(Keywords: regression)

Attachments

(2 files, 3 obsolete files)

Add --enable-application=mail|im|suite support to mozilla-central
1.82 KB, patch
glandium
: review+
Details | Diff | Splinter Review
Rough port of bug 943732
11.74 KB, patch
standard8
: review+
Details | Diff | Splinter Review 
If the "mk_add_options MOZ_OBJDIR" make option in .mozconfig is set absolute and not relative, the configure options are ignored.

steps to reproduce:
1. set MOZ-OBJDIR to e.g. /home/yourname/objdir
2. start configure

expected results:
configure options are shown when evaluted

actual result:
configure options are ignored.
Keywords: regression
Note this might be a fairly recent regression, or a platform specific issue. I recently did an update when m-c was at 161fadfcbadc (July 12th) and I am using an absolute path on Mac. Lightning was built for me with --enable-calendar.
Markus, is your objdir inside your comm-central directory or somewhere else?
I was wondering why builds took so long. 
about:buildconfig doesn't mention --disable-optimize which i normally use (and saves a lot of time)
objdir resides in @TOPSRCDIR@/../objdir also when I set it with an absolute path.
You might be seeing bug 1036667?
My objdir is at an external location.
(In reply to Magnus Melin from comment #5)
> You might be seeing bug 1036667?
> My objdir is at an external location.

markus@ubuntu:~$ echo $MOZCONFIG

markus@ubuntu:~$

I don't think so...
Just for comparison, could you try an absolute objdir that is inside your comm-central directory?
absolute dir inside comm-central works.
Blocks: 1034708
What does blocking Bug 1035096 mean here, that it's a regression from that bug?
(In reply to Frank Wein [:mcsmurf] from comment #9)
> What does blocking Bug 1035096 mean here, that it's a regression from that
> bug?

Yes.
No longer blocks: 1034708
With this change, it's possible to simplify Thunderbird's configure to essentially mozilla/configure "$@" --with-external-source-dir="$(dirname $0)".
Assignee: nobody → Pidgeot18
Status: NEW → ASSIGNED
Attachment #8467454 - Flags: review?(mh+mozilla)
This works locally on Linux. It busts automation horribly, though, since automation is explicitly looking for a mozilla/ directory.
Attachment #8467467 - Flags: review?(standard8)
Comment on attachment 8467454 [details] [diff] [review]
Add --enable-application=mail|im|suite support to mozilla-central

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

::: configure.in
@@ +3908,5 @@
>      AC_MSG_ERROR([--enable-application=mobile is no longer supported.])
>    fi
> +  # Support comm-central.
> +  if test -n "$EXTERNAL_SOURCE_DIR" ; then
> +    MOZ_BUILD_APP="../$MOZ_BUILD_APP"

Why not $EXTERNAL_SOURCE_DIR/$MOZ_BUILD_APP?
(In reply to Mike Hommey [:glandium] from comment #14)
> Comment on attachment 8467454 [details] [diff] [review]
> Add --enable-application=mail|im|suite support to mozilla-central
> 
> Review of attachment 8467454 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: configure.in
> @@ +3908,5 @@
> >      AC_MSG_ERROR([--enable-application=mobile is no longer supported.])
> >    fi
> > +  # Support comm-central.
> > +  if test -n "$EXTERNAL_SOURCE_DIR" ; then
> > +    MOZ_BUILD_APP="../$MOZ_BUILD_APP"
> 
> Why not $EXTERNAL_SOURCE_DIR/$MOZ_BUILD_APP?

because MOZ_BUILD_APP needs to be relative in three lines, and EXTERNAL_SOURCE_DIR is absolute.
(In reply to Joshua Cranmer [:jcranmer] (high latency until August 11) from comment #15)
> > Why not $EXTERNAL_SOURCE_DIR/$MOZ_BUILD_APP?
> 
> because MOZ_BUILD_APP needs to be relative in three lines, and
> EXTERNAL_SOURCE_DIR is absolute.


I'd rather see a modification to that than to hardcode "../", which rubs me in the wrong direction: if you're hardcoding "..", why do we have $EXTERNAL_SOURCE_DIR in the first place and not a flag that enforces it to be ".."?
Comment on attachment 8467454 [details] [diff] [review]
Add --enable-application=mail|im|suite support to mozilla-central

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

See comment 16.
Attachment #8467454 - Flags: review?(mh+mozilla) → review-
If only using os.path.relpath weren't so bloody hard in configure.
Attachment #8467454 - Attachment is obsolete: true
Attachment #8472488 - Flags: review?(mh+mozilla)
Does Bug 1053071 impact on this?
Flags: needinfo?(Pidgeot18)
(In reply to Ian Neal from comment #19)
> Does Bug 1053071 impact on this?

I looked at that; it shouldn't (it's just changing some not-useful genericity in moz.build).
Flags: needinfo?(Pidgeot18)
Meep, I needed another change to get 32-bit Linux working. It seems that the only way to get LDAP C-SDKS to pick up that the CXX is g++ -m32 is by using a cache file, and this is the easiest way to get it that cache file.
Attachment #8472488 - Attachment is obsolete: true
Attachment #8472488 - Flags: review?(mh+mozilla)
Attachment #8472627 - Flags: review?(mh+mozilla)
Comment on attachment 8472627 [details] [diff] [review]
Add --enable-application=mail|im|suite support to mozilla-central

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

::: configure.in
@@ +3915,5 @@
>    fi
> +  # Support comm-central.
> +  if test -n "$EXTERNAL_SOURCE_DIR" ; then
> +    MOZ_BUILD_APP="$EXTERNAL_SOURCE_DIR/$MOZ_BUILD_APP"
> +    MOZ_BUILD_APP=`${PYTHON} -c "import os.path; print(os.path.relpath(\"${MOZ_BUILD_APP}\", \"${srcdir}\"))"`

That seems awful for something that could be handled by adding another test without ${srcdir} below.

@@ +9047,5 @@
>    _save_srcdir="$srcdir"
>    srcdir="$srcdir/.."
>    _save_ac_configure_args="$ac_configure_args"
>    ac_configure_args="$_subconfigure_config_args"
> +  AC_OUTPUT_SUBDIRS("$_subconfigure_subdir",$cache_file)

That's a separate issue, though.
Attachment #8472627 - Flags: review?(mh+mozilla) → review-
Comment on attachment 8472627 [details] [diff] [review]
Add --enable-application=mail|im|suite support to mozilla-central

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

After discussion on IRC, we determined that MOZ_BUILD_APP needs to be relative for moz.build anyways, so I'm re-requesting review on the patch.

The configure change for LDAP is because I don't think it's really worth making two patches for two one-liners.
Attachment #8472627 - Flags: review- → review?(mh+mozilla)
This contains the automation hack as well because, well, it kind of keeps the build working on Try and comm-central without needing to change the buildbot rules.
Attachment #8467467 - Attachment is obsolete: true
Attachment #8467467 - Flags: review?(standard8)
Attachment #8473412 - Flags: review?(standard8)
Comment on attachment 8473412 [details] [diff] [review]
Rough port of bug 943732

Ok, I've had a quick look at it and it seems fine.
Attachment #8473412 - Flags: review?(standard8) → review+
Attachment #8472627 - Flags: review?(mh+mozilla) → review+
Pushed to mozilla-central and comm-central:
https://hg.mozilla.org/mozilla-central/rev/85135c5c6ba8
https://hg.mozilla.org/comm-central/rev/8f57b0340428
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 34.0
Blocks: 1059511
Comment on attachment 8473412 [details] [diff] [review]
Rough port of bug 943732

>+# If we have a MOZ_OBJDIR that's set from the environment, ensure that it is an
>+# absolute path.
>+ifdef MOZ_OBJDIR
>+MOZ_OBJDIR := $(shell $(PYTHON) -c "import os.path; print(os.path.join(\"$(TOPSRCDIR)\", \"$(MOZ_OBJDIR)\").replace('\\\\','/'))")
>+endif
I've been using MOZ_OBJDIR=. to refer to $(PWD) for almost exactly 4 years now, given that it's hardcoded on line 53...
Comment on attachment 8473412 [details] [diff] [review]
Rough port of bug 943732

>+if test "$MOZ_AUTOMATION" -eq 1; then
This warns if $MOZ_AUTOMATION isn't an integer expression...
Blocks: 1060959
Depends on: 1068100
Depends on: 1053761
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Hi all, this change broke quite a lot of ESR builds downstream.

We used to set objdir via mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/objdir in mozconfig.
Now, with 31.3.0 this fails shortly after the build started:


make[8]: Nothing to be done for 'export'.
make[8]: Leaving directory '/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/objdir/mozilla/dom/network/src'
make[7]: Leaving directory '/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/objdir/mozilla/dom/network'
make[6]: Leaving directory '/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/objdir/mozilla/dom/network'
Traceback (most recent call last):
  File "/usr/lib/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/mozilla/python/mozbuild/mozbuild/action/webidl.py", line 17, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/mozilla/python/mozbuild/mozbuild/action/webidl.py", line 12, in main
    manager = BuildSystemWebIDL.from_environment().manager
  File "/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/mozilla/python/mozbuild/mozbuild/base.py", line 198, in from_environment
    raise ObjdirMismatchException(topobjdir, _config_topobjdir)
mozbuild.base.ObjdirMismatchException: Objdir mismatch: /home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/objdir/mozilla != /home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/mozilla/objdir
Makefile:73: recipe for target 'codegen.pp' failed
make[6]: *** [codegen.pp] Error 1
make[6]: Leaving directory '/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/objdir/mozilla/dom/bindings'
/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/mozilla/config/recurse.mk:179: recipe for target 'bindings_export' failed
make[5]: *** [bindings_export] Error 2
make[5]: Leaving directory '/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/objdir/mozilla/dom'
/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/mozilla/config/recurse.mk:179: recipe for target 'export' failed
make[4]: *** [export] Error 2
make[4]: Leaving directory '/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/objdir/mozilla/dom'
/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/mozilla/config/recurse.mk:153: recipe for target 'export' failed
make[3]: *** [export] Error 2
make[3]: Leaving directory '/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/objdir/mozilla'
/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/mozilla/config/rules.mk:592: recipe for target 'default' failed
make[2]: *** [default] Error 2
make[2]: Leaving directory '/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/objdir/mozilla'
Makefile:52: recipe for target 'default' failed
make[1]: *** [default] Error 2
make[1]: Leaving directory '/home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/objdir'
client.mk:391: recipe for target 'build' failed
make: *** [build] Error 2
error: Bad exit status from /home/iurt/rpmbuild/tmp/rpm-tmp.xoJoFJ (%build)


I believe this was introduced by the changes discussed in this bug, culprit is:

mozbuild.base.ObjdirMismatchException: Objdir mismatch: /home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/objdir/mozilla != /home/iurt/rpmbuild/BUILD/thunderbird-31.3.0/comm-esr31/mozilla/objdir
(In reply to doktor5000 from comment #29)
> Hi all, this change broke quite a lot of ESR builds downstream.

I'm interested to know why you think a patch that landed in the 34 timeframe and hasn't been uplifted since would break a build based on 31.

Nevertheless, reopening a bug because it is broken only on a branch is not the right thing to do.
Status: REOPENED → RESOLVED
Closed: 9 years ago → 8 years ago
Resolution: --- → FIXED
(In reply to Joshua Cranmer [:jcranmer] from comment #30)
> Nevertheless, reopening a bug because it is broken only on a branch is not
> the right thing to do.

Sorry, didn't find a clone function here in bugzilla.
Opened https://bugzilla.mozilla.org/show_bug.cgi?id=1106883 and put you on CC.
You need to log in before you can comment on or make changes to this bug.