Closed
Bug 1035599
Opened 10 years ago
Closed 10 years ago
Pseudo-merge m-c and c-c's objdir
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla34
People
(Reporter: glandium, Assigned: jcranmer)
References
Details
Attachments
(3 files, 3 obsolete files)
881 bytes,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
14.56 KB,
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
28.54 KB,
patch
|
jcranmer
:
review+
|
Details | Diff | Splinter Review |
Essentially a dump if anyone wants to take it further, because I won't. The attached patch makes the build work for me on linux by doing the following:
- Create objdir
- Run m-c's configure with --enable-application=../mail --with-external-source-dir=..
There is no actual integration with the c-c build system, and there are very likely issues with mozconfigs, as this makes the topsrcdir of the build is m-c.
Note that on c-c's end, this requires this patch to build: https://bugzilla.mozilla.org/attachment.cgi?id=8376652
Reporter | ||
Comment 1•10 years ago
|
||
Note it should be possible to go without the recursivemake.py changes with more changes on c-c's end, and that would make the c-c tree use the m-c config/*.mk.
Assignee | ||
Comment 2•10 years ago
|
||
Okay, since I happen to unexpectedly have a bit of time on my hands at the moment, I'm going to try pushing forward on this.
Originally, I wanted to tweak these patches so that $(topsrcdir) pointed to mozilla-central instead of comm-central. That turned out to be a bit harder than I expected, so I'll separate that step into another patch.
Cc'ing a few people who may want to be aware that this will likely require investigation on their part to make sure that builders keep working.
Assignee | ||
Comment 3•10 years ago
|
||
This also ought to fix the issue with Windows and OS X builds with the /Makefile.in change.
Assignee: nobody → Pidgeot18
Attachment #8452099 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8466875 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 4•10 years ago
|
||
Parts of this patch are leveraged from the cc-rework bug, particularly the changes for packaging tests and some of the global path renamings.
I have not tested that SeaMonkey and Instantbird work with this patch. I also haven't test l10n repacks because I have no clue how to run those.
On the other hand, this appears to solve all outstanding build issues on comm-central.
Attachment #8466878 -
Flags: review?(standard8)
Comment 5•10 years ago
|
||
Comment on attachment 8466878 [details] [diff] [review]
Comm-central changes
Review of attachment 8466878 [details] [diff] [review]:
-----------------------------------------------------------------
Ok, I can't get this to build locally, mainly due to bug 1040009. However the changes & the fact there's a try build seem to indicate its certainly a lot better than what we have. We can monitor l10n builds as we go.
Attachment #8466878 -
Flags: review?(standard8) → review+
Assignee | ||
Comment 6•10 years ago
|
||
A little bit more m-c changes to fix the inclusion of the branding configure.sh.
Attachment #8467390 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8466875 -
Flags: review?(mh+mozilla) → review?(gps)
Reporter | ||
Updated•10 years ago
|
Attachment #8467390 -
Flags: review?(mh+mozilla) → review+
Reporter | ||
Comment 7•10 years ago
|
||
Comment on attachment 8466875 [details] [diff] [review]
Mozilla-central changes
Review of attachment 8466875 [details] [diff] [review]:
-----------------------------------------------------------------
::: Makefile.in
@@ +327,5 @@
> ifeq ($(MOZ_WIDGET_TOOLKIT),gtk3)
> toolkit/library/target: widget/gtk/mozgtk/gtk3/target
> endif
> ifdef MOZ_LDAP_XPCOM
> +ldap/target: security/build/target mozglue/build/target
isn't this the fix for a separately filed bug?
Reporter | ||
Comment 8•10 years ago
|
||
> Flags: review?(mh+mozilla@glandium.org) → review?(gps@mozilla.com)
In case it isn't clear why, this was discussed on irc, and the reason is that the patch is largely mine (attachment 8452099 [details] [diff] [review]) rebased, with a Makefile.in change.
Comment 9•10 years ago
|
||
Comment on attachment 8466875 [details] [diff] [review]
Mozilla-central changes
Review of attachment 8466875 [details] [diff] [review]:
-----------------------------------------------------------------
This is on the right track, but not quite ready for r+.
::: configure.in
@@ +2523,5 @@
> ;;
> *)
> case $GCC_VERSION in
> 4.4*|4.6*)
> + VISIBILITY_FLAGS='-I$(DIST)/system_wrappers -include $(MOZILLA_DIR)/config/gcc_hidden_dso_handle.h'
Does MOZILLA_DIR exist in m-c?
::: python/mozbuild/mozbuild/frontend/reader.py
@@ +139,5 @@
> topobjdir = mozpath.abspath(config.topobjdir)
> topsrcdir = config.topsrcdir
> norm_topsrcdir = mozpath.normpath(topsrcdir)
>
> + self.externals = []
Could we make this self.external_source_dirs? "externals" feels a bit ambiguous to me.
@@ +205,4 @@
> """
> if os.path.isabs(path):
> + if filesystem_absolute:
> + return path
Do we need to normalize the returned path?
@@ +206,5 @@
> if os.path.isabs(path):
> + if filesystem_absolute:
> + return path
> + for root in [self.topsrcdir] + self.externals:
> + p = mozpath.normpath(mozpath.join(root, path[1:]))
Please add a comment that the [1:] is to work around .join()'s behavior of ignoring previous components if an absolute path is encountered.
Also, what happens if path is a Windows style path? Perhaps we should mozpath.normpath(path) or at least assert path[0] == '/'
@@ +209,5 @@
> + for root in [self.topsrcdir] + self.externals:
> + p = mozpath.normpath(mozpath.join(root, path[1:]))
> + if os.path.exists(p):
> + return p
> + return mozpath.normpath(mozpath.join(self.topsrcdir, path[1:]))
Ditto.
@@ +228,4 @@
>
> # realpath() is needed for true security. But, this isn't for security
> # protection, so it is omitted.
> + normalized_path = self.normalize_path(path, filesystem_absolute)
Nit: filesystem_absolute should be passed as a named argument, not as a positional argument.
@@ +883,5 @@
> 'Tier directory (%s) registered multiple '
> 'times in %s' % (d, tier), sandbox)
> recurse_info[d] = {'tier': tier,
> 'parent': sandbox['RELATIVEDIR'],
> + 'var': 'TIER_DIRS'}
TIER_DIRS doesn't exist.
Attachment #8466875 -
Flags: review?(gps) → feedback+
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #9)
> Does MOZILLA_DIR exist in m-c?
Yes.
> @@ +205,4 @@
> > """
> > if os.path.isabs(path):
> > + if filesystem_absolute:
> > + return path
>
> Do we need to normalize the returned path?
According to glandium, no. The function will only be called with purprortedly topsrcdir-relative-with-/ paths.
Assignee | ||
Comment 11•10 years ago
|
||
Attachment #8466875 -
Attachment is obsolete: true
Attachment #8468157 -
Flags: review?(gps)
Reporter | ||
Comment 12•10 years ago
|
||
(In reply to Joshua Cranmer [:jcranmer] (high latency until August 11) from comment #10)
> (In reply to Gregory Szorc [:gps] from comment #9)
> > Does MOZILLA_DIR exist in m-c?
>
> Yes.
> > @@ +205,4 @@
> > > """
> > > if os.path.isabs(path):
> > > + if filesystem_absolute:
> > > + return path
> >
> > Do we need to normalize the returned path?
>
> According to glandium, no. The function will only be called with
> purprortedly topsrcdir-relative-with-/ paths.
Which, after reading further, I figured is not true, but it doesn't matter anyways. The function is either called with a filesystem absolute path in which case the path we do want is the path that was given in, or a topsrcdir-absolute-or-relative path with forward slashes, in which case, it's mozpath.joined top the relevant directory (and in which case path[1:] works because it's a topsrcdir-absolute path, never a windows path.
The result of that function is expected to be a system path, not a moz.build path, so it needs no normalization, and the required normalization is done many times by whatever uses the result of the function.
Updated•10 years ago
|
Attachment #8468157 -
Flags: review?(gps) → review+
Assignee | ||
Updated•10 years ago
|
Attachment #8467390 -
Flags: checkin+
Assignee | ||
Updated•10 years ago
|
Attachment #8468157 -
Flags: checkin+
Assignee | ||
Comment 13•10 years ago
|
||
Assignee | ||
Comment 14•10 years ago
|
||
This is the patch I intend to check in when m-i merges to m-c. It's fixed for the bitrot that glandium added, and has a few other changes to make Instantbird and Seamonkey build, approved by Florian and IanN.
Attachment #8466878 -
Attachment is obsolete: true
Attachment #8469729 -
Flags: review+
Comment 15•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e234b61f711e
https://hg.mozilla.org/mozilla-central/rev/698584a65cdb
https://hg.mozilla.org/mozilla-central/rev/7dc089c4bd76
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Assignee | ||
Comment 16•10 years ago
|
||
And here's the c-c changeset:
https://hg.mozilla.org/comm-central/rev/949e0465b1c3
Comment 17•10 years ago
|
||
(In reply to Joshua Cranmer [:jcranmer] from comment #16)
> And here's the c-c changeset:
> https://hg.mozilla.org/comm-central/rev/949e0465b1c3
Fix for Mac universal builds: https://hg.mozilla.org/comm-central/rev/cb779d2d4181
Comment 18•10 years ago
|
||
Fixed a few issues in im/build.mk: https://hg.mozilla.org/comm-central/rev/3c377dcfb242
Updated•10 years ago
|
QA Whiteboard: [qa-]
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•