Closed
Bug 395019
Opened 17 years ago
Closed 12 years ago
compute relativesrcdir from srcdir, topsrcdir
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: ted, Assigned: joey)
References
()
Details
Attachments
(2 files, 2 obsolete files)
2.00 KB,
text/plain
|
Details | |
6.23 KB,
patch
|
Details | Diff | Splinter Review |
relativesrcdir gets set in a lot of Makefiles. It seems like we could just compute it, perhaps in autoconf.mk, like so: relativesrcdir = $(subst $(topsrcdir)/,,$(srcdir)) We'll need to test this and make sure it works consistently.
Reporter | ||
Comment 1•17 years ago
|
||
I tested this on MSYS, Cygwin, Linux, and OSX, and it seems to work fine on all of them.
Attachment #280387 -
Flags: review?(benjamin)
Updated•17 years ago
|
Attachment #280387 -
Flags: review?(benjamin) → review+
Reporter | ||
Updated•17 years ago
|
Attachment #280387 -
Flags: approval1.9?
Comment 2•17 years ago
|
||
Comment on attachment 280387 [details] [diff] [review] simple fix in autoconf.mk.in [Backed out: Comment 7] a=bzbarsky Can we start removing existing sets of this, then?
Attachment #280387 -
Flags: approval1.9? → approval1.9+
Reporter | ||
Comment 3•17 years ago
|
||
Yes. I didn't think it was worth bloating up this tiny patch for that, since the existing ones shouldn't hurt anything, but we could take them out all at once or in pieces, whichever works. Checked in.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
It caused this error on Linux and Solaris. gmake[5]: Entering directory `/export/home/mrbld/tinderbox/SunOS_5.11_Depend/mozilla/netwerk/locales' error: file '../.././en-US/necko.properties' doesn't exist at ../../config/make-jars.pl line 462, <STDIN> line 4. +++ making chrome /export/home/mrbld/tinderbox/SunOS_5.11_Depend/mozilla/netwerk/locales => ../../dist/bin/chrome/en-US.jar gmake[5]: *** [libs] Error 2 gmake[5]: Leaving directory `/export/home/mrbld/tinderbox/SunOS_5.11_Depend/mozilla/netwerk/locales' gmake[4]: *** [libs] Error 2
on my box netwerk/locales/Makefile has topsrcdir = ../.. srcdir = . so relativesrcdir = $(subst $(topsrcdir)/,,$(srcdir)) i.e. relativesrcdir = . config.mk Line 864 will not work because it tries $(topsrcdir)/$(relativesrcdir)/en-US as ../.././en-US same problem at dom/locales Reopening this bug, since the computation is not correct.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 6•17 years ago
|
||
It seems to me that make-makefile could be used to substitute this in.
Reporter | ||
Comment 7•17 years ago
|
||
Comment on attachment 280387 [details] [diff] [review] simple fix in autoconf.mk.in [Backed out: Comment 7] Alright, I backed this out for now. Ginn: what does your build environment look like? Do you have an objdir set in your mozconfig?
Attachment #280387 -
Attachment is obsolete: true
Reporter | ||
Comment 8•17 years ago
|
||
(In reply to comment #6) > It seems to me that make-makefile could be used to substitute this in. That would certainly be doable, but I was hoping for a simpler solution. I guess it might not be possible.
Reporter | ||
Comment 9•17 years ago
|
||
FWIW, I think this is only broken for srcdir builds, unless someone knows of another case where this can happen.
Comment 10•17 years ago
|
||
Yes, I'm building in srcdir. The following line works for me. But I didn't test on any other platform, or configure/build in another dir. relativesrcdir = $(subst $(shell cd $(topsrcdir); pwd)/,,$(shell cd $(srcdir); pwd))
Comment 11•17 years ago
|
||
Comment on attachment 280387 [details] [diff] [review] simple fix in autoconf.mk.in [Backed out: Comment 7] clearing approval on obsolete patch. please ask again for a new patch
Attachment #280387 -
Flags: approval1.9+
Comment 12•15 years ago
|
||
When the autoconf.mk part is fixed (here), I can do the cleanup part (while doing bug 479998) ;-)
Blocks: 479998
Status: REOPENED → NEW
Comment 13•15 years ago
|
||
There are 2 possibilities to fix these 4 occurrences: 1) Move the |relativesrcdir| setting after autoconf.mk inclusion to override the global definition. Less work, but leaves exceptions. 2) Adapt them to match the expected value/location. More work, but no exceptions left. The choice is yours. ***** (In reply to comment #10) > relativesrcdir = $(subst $(shell cd $(topsrcdir); pwd)/,,$(shell cd $(srcdir); > pwd)) Do we want to do it exactly like this ? Or is calling the shell (twice) considered too expansive and we would want to try the simple substitution first and only do this as a fallback?
Assignee: ted.mielczarek → sgautherie.bz
Status: NEW → ASSIGNED
Comment 14•15 years ago
|
||
The complete one, this time! It looks like all the tests could be fixed to match the new global rule. (with the possibility to use an override if some can't be adapted.) To be safe with the locales, I would suggest to rename the variable they use to |localerelativesrcdir| for example. What/How do you think we should do?
Updated•15 years ago
|
Attachment #371040 -
Attachment is obsolete: true
Comment 15•15 years ago
|
||
(In reply to comment #13) > > relativesrcdir = $(subst $(shell cd $(topsrcdir); pwd)/,,$(shell cd $(srcdir); > > pwd)) > > Do we want to do it exactly like this ? > Or is calling the shell (twice) considered too expansive and we would want to > try the simple substitution first and only do this as a fallback? (In reply to comment #14) > To be safe with the locales, I would suggest to rename the variable they use to > |localerelativesrcdir| for example. > > What/How do you think we should do? Ted, how should we proceed on this bug?
Reporter | ||
Comment 16•15 years ago
|
||
We're not putting two shell calls in every makefile just to fix this. I think the only safe way to fix this is to make make-makefile know how to produce this, and then start sticking "relativesrcdir = @relativesrcdir@" in makefiles.
Comment 17•15 years ago
|
||
(In reply to comment #5) (1.5 years later:) > on my box I tried on my local W2K with and without |mk_add_options MOZ_OBJDIR=...| and got the same full paths in both cases... > netwerk/locales/Makefile has > topsrcdir = ../.. > srcdir = . topsrcdir = [...]/mozilla srcdir = [...]/mozilla/netwerk/locales > same problem at dom/locales topsrcdir = [...]/mozilla srcdir = [...]/mozilla/dom/locales I asked Ted who replied "it's possible something changed". Ginn, or anyone, would you know how to reproduce the '. and ..' output? (NB: make-makefile seems to still have code to handle "dots"...) Otherwise, I would suggest to check Ted's patch in again.
Comment 18•15 years ago
|
||
(In reply to comment #14) > Non-matching |relativesrcdir| value list Is the following a problem or not ? { 825 ifdef relativesrcdir 826 LOCALE_SRCDIR = $(call EXPAND_LOCALE_SRCDIR,$(relativesrcdir)) 827 endif } > To be safe with the locales, I would suggest to rename the variable they use to > |localerelativesrcdir| for example. Ted, what about these two questions?
Updated•14 years ago
|
Updated•12 years ago
|
Attachment #280387 -
Attachment description: simple fix in autoconf.mk.in → simple fix in autoconf.mk.in
[Backed out: Comment 7]
Comment 19•12 years ago
|
||
3 years without answers, giving up.
Assignee: sgautherie.bz → nobody
Status: ASSIGNED → NEW
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → joey
Assignee | ||
Comment 20•12 years ago
|
||
Assignee | ||
Comment 21•12 years ago
|
||
Comment on attachment 632703 [details] [diff] [review] compute relativesrcdir from srcdir, topsrcdir Added makefile library function getRelSrcDir able to compute paths from topsrcdir and srcdir. Usage: relpath = $(call getRelSrcDir) Function will verify topsrcdir and srcdir both contain a value. $(relpath ) is used to normalize paths (~symlinks). $(or ) used to detect non-existent/yet to be created paths. Unit test will verify subdir extraction from: o absolute paths o ../.., ./ o non-existent, non-overlapping paths
Attachment #632703 -
Flags: review?(ted.mielczarek)
Comment 22•12 years ago
|
||
Note that realpath requires make 3.81 and we currently require 3.80.
Comment 23•12 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #22) > Note that realpath requires make 3.81 and we currently require 3.80. 3.81 was released August 1, 2006. Just saying ;)
Assignee | ||
Comment 24•12 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #23) > (In reply to Mike Hommey [:glandium] from comment #22) > > Note that realpath requires make 3.81 and we currently require 3.80. > > 3.81 was released August 1, 2006. Just saying ;) Do we still have platforms dependent on v3.80 or has the minimum requirement just not been updated for a while ? Even make bundled with mozilla-build(~msvc9) appears to have been patched to a newer version: $ make --version GNU Make 3.81.90 This program built for i686-pc-msys32
Reporter | ||
Comment 25•12 years ago
|
||
Comment on attachment 632703 [details] [diff] [review] compute relativesrcdir from srcdir, topsrcdir Review of attachment 632703 [details] [diff] [review]: ----------------------------------------------------------------- Sorry, I ran out of time trying to get reviews done today. Feel free to punt this to someone else or I can look at it when I'm back from vacation.
Reporter | ||
Comment 26•12 years ago
|
||
Comment on attachment 632703 [details] [diff] [review] compute relativesrcdir from srcdir, topsrcdir glandium's mega-patch-series in bug 774032 is taking care of this in a different manner, which is probably the better way to go in the long run.
Attachment #632703 -
Flags: review?(ted.mielczarek)
Reporter | ||
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 17 years ago → 12 years ago
Resolution: --- → WONTFIX
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•