Closed
Bug 278132
Opened 20 years ago
Closed 3 years ago
NSS makefile dependencies broken, mkdepend missing
Categories
(NSS :: Build, defect, P5)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: nelson, Assigned: nelson)
References
Details
Attachments
(2 files, 2 obsolete files)
The "coreconf" system of Makefiles has extensive support for dynamic contruction of makefile dependencies. grep the *.mk files in coreconf for the string MKDEPEND to see that support. The idea is that one pulls the sources, then in the nss directory, one makes the "export" target, and then makes the "depend" target. This creates a dependency file in each directory (IIRC). However, this feature depends on a program named mkdepend. The coreconf directory has a subdirectory named mkdepend. Prior to the opening of the NSS source code, that directory contained the portable source to the program that generated the makefile dependencies. Makeing the target "all" in coreconf, or making the target build_coreconf in nss, would build the mkdepend program, and thereafter, making the "depend" target would create the dependnecies files. This worked on all unix/linux platforms and on windows. When the NSS source was opened, the source for mkdepend was not carried forward into the open source repository due to some concerns about license. IIRC, the mkdepend source has a BSD license, and someone thought we could or should not put such source into the mozilla/security tree, but rather ALL source in the mozilla tree should be MPLed. Consequently, the makefile dependencies for NSS have been broken ever since the source was opened. :( If possible, I would like to see mkdepend restored to coreconf, perhaps with its BSD license intact. Comments? Opinions?
Comment 1•20 years ago
|
||
This is fine.
Assignee | ||
Updated•18 years ago
|
QA Contact: wtchang → build
Assignee | ||
Comment 3•18 years ago
|
||
Gerv, any comments on the proposal to put the c sources for the "mkdepend" program into NSS, with the original BSD license statements intact (and no MPL boilerplate) ?
Comment 4•18 years ago
|
||
Could someone post a copy of the licence here? There's almost certainly no problem with this (we have other BSD code in the tree) but I'd like to check to be sure. Gerv
Assignee | ||
Updated•18 years ago
|
Priority: -- → P5
make seamonkeey-1.0.7 elimate this error: make[3]: Entering directory `/src/src/mozilla/security/coreconf' syntax error at -e line 3, near "while" syntax error at -e line 7, near "}" Execution of -e aborted due to compilation errors. which is casued by coreconf/rules.mk, line open(MD,"< $(DEPENDENCIES)") i guess the make variable $(DENPENDENCIES) was replaced by something like '?/.md', but the file cannot be found. also config/mkdepend has nothing but a Makefile, that means the mkdepend command was not performed, in chain, the .md file was not created. so i wanna is this a bug or i missed something? i'm building in a seperate dir other than source dir. thanks.
Assignee | ||
Comment 6•18 years ago
|
||
Comment 5 is irrelevant to this bug. See bug 325148. Please add comments to the appropriate bug.
Comment 7•16 years ago
|
||
(In reply to comment #4) > Could someone post a copy of the licence here? There's almost certainly no > problem with this (we have other BSD code in the tree) but I'd like to check to > be sure. The source code is already present at mozilla/config/mkdepend I conclude it's not a problem to add another copy to NSS (if we intend that NSS has its own dep system, independent of the mozilla build).
Comment 8•16 years ago
|
||
This bug should be fixed with a high priority. Last Friday I tried to upgrade Firefox to a newer snapshot, and because of the missing dependencies all the tinderbox build machines failed.
Comment 9•16 years ago
|
||
During the last couple of days Nelson and I worked on this. According to my test, and repeating my "landing experience" locally, the dependency system fixed it for me. (With changes in my local tree to make NSS dependencies work, and having copied the mkdepend executable to the path.) The trouble so far is building the mkdepend.exe itself as part of the NSS build process. While it easily builds on Linux, more work is necessary to get it built as part of the NSS build process. We learned that building on Windows requires knowledge from the autoconfig-run produced in mozilla/config/autoconf.mk, for example we need -DNO_X11 and according to Nelson we need even more. So, a solution to this bug requires: - copy mozilla/config/mkdepend to mozilla/security/coreconf/mkdepend - make changes to the NSS build system to allow building of mkdepend on all platforms - make changes to the NSS makefiles to run mkdepend and use its output
Assignee | ||
Comment 10•15 years ago
|
||
I decided to look at what it would take to get mkdepend back into coreconf. The original issues were: a) licensing - tri-license versus MIT or BSD. b) Windows - it just didn't work on Windows very well or at all. So I took at stab at this. I copied the current mkdepend files from mozilla/config/mkdepend to mozilla/security/coreconf/mkdepend, and went to work making it build and work well on Windows. I have that working now. Here is an explanation of the attached patch. This patch appears to be a patch to the code in mozilla/config/mkdepend, but I do NOT propose to change any code in that directory. Instead, I propose to copy the files from mozilla/config/mkdepend to mozilla/security/coreconf/mkdepend and commit them, making coreconf's copy be a copy of the other one. THEN I propose to apply this patch to the files in mozilla/security/coreconf/mkdepend, I know this works when built with MSVC on Windows XP (Win32). I haven't yet checked to see if it is satisfactory on other platforms. I'd appreciate help with that testing.
Assignee | ||
Comment 11•15 years ago
|
||
Comment on attachment 361923 [details] [diff] [review] patch v 0.1 I invite review comments from anyone who cares, especially Wan-Teh.
Attachment #361923 -
Flags: review?(wtc)
Updated•15 years ago
|
Attachment #361923 -
Flags: review?(wtc) → review+
Comment 12•15 years ago
|
||
Comment on attachment 361923 [details] [diff] [review] patch v 0.1 r=wtc. 1. Makefile >+CFLAGS += -DNO_X11 In coreconf, it's better to add -DXXX to DEFINES instead. >+CFLAGS += -DINCLUDEDIR=\"/usr/include\" -DOBJSUFFIX=\".$(OBJ_SUFFIX)\" Same as above -- it's better to add these to DEFINES instead. Also, /usr/include is a Unix specific pathname. 2. Makefile.in >-ifdef GNU_CC >-MODULE_OPTIMIZE_FLAGS = -O3 >-else >-ifeq ($(OS_ARCH),SunOS) >-MODULE_OPTIMIZE_FLAGS = -fast >-endif >-endif >- >-ifeq ($(OS_ARCH),WINNT) >-ifndef GNU_CC >-MODULE_OPTIMIZE_FLAGS = -Ox >-endif >-endif We probably should do the same thing (enable compiler optimizations) in Makefile. >-ifdef GNU_CC >-_GCCDIR = $(shell $(CC) -print-file-name=include) >-HOST_CFLAGS += -DPREINCDIR=\"$(_GCCDIR)\" >-endif You didn't carry this over to Makefile. Not sure what the consequences would be. >-$(HOST_OBJS): def.h ifparser.h imakemdep.h We may want this. 3. imakemdep.h Do we really need to list all the compiler predefined macros? It's impossible to list all the predefined macros for all compilers on all platforms. _X86, _X86_, _X86_64, and _X86_64_ may not be predefined macros. They may be defined by a Windows system header. 4. main.c > if (bol && whitespace) { >+ if (!strstr(filep->f_name, INCLUDEDIR)) > warning("%s: non-portable whitespace encountered at line %d\n", > filep->f_name, lineno); > } I don't understand this change. Also, rather than using nested ifs, you should add the new test to the original if: if (bol && whitespace && !strstr(filep->f_name, INCLUDEDIR)) {
Assignee | ||
Comment 13•15 years ago
|
||
Wan-Teh, In reply to comment 12, 1. s/FLAGS/DEFINES/ OK, I'll try it, and if it works, I'll do that 2. optimization I removed that stuff because it uses make variables that are not used in coreconf. I can look into ensuring that mkdepend is built with optimizations for gcc, MSVC and solaris 3. I was unable to get most code to build on Windows until I defined many of MSVC's pre-defined symbols. I found Microsoft's official list of them at http://msdn.microsoft.com/en-us/library/b0084kay.aspx 4. about INCLUDEDIR: - yes, /usr/include is a Unix location - INCLUDEDIR is not used at all on WINDOWS or OS2. On those platforms, the locations of the system's root include directories are specified in environment variables. - mkdepend is derived from lint, and its display of the warning about lines whose first non-blank character is a '#' after column 1 is a lint-ish warning. Frankly, I think that such warnings have no place in mkdepend at all, but I absolutely see no point in having it complain about such issues in SYSTEM header files. So, the line I added, which calls strstr, determines if the source file being used comes from INCLUDEDIR, and if so, it suppresses the lint warning. Note that this entire section of code (quoted above) is ifdeff'ed out on Windows because trying to identify the system include directories on Windows and OS2 is not easy, and would require a MUCH bigger change. Perhaps I should simply remove the section of code quoted above entirely on all platforms. Would you prefer that?
Assignee | ||
Comment 14•15 years ago
|
||
Checking in Makefile; initial revision: 1.1 Checking in imakemdep.h; new revision: 1.3; previous revision: 1.2 Checking in main.c; new revision: 1.3; previous revision: 1.2
Comment 15•15 years ago
|
||
On second thought, I think it's OK to compile mkdepend without optimizations because the NSS source tree is small.
Comment 16•15 years ago
|
||
Comment on attachment 362341 [details] [diff] [review] patch as checked in Should we contribute your changes to imakemdep.h and main.c back to mozilla/config/mkdepend or the latest copy in mozilla-central? I'll compare what you checked in to coreconf with the copy in mozilla-central. It's still not clear why the predefined macros need to be listed, but you don't need to explain it to me. Do we need to list, for example, all the predefined macros of GCC and Sun Studio 12? http://gcc.gnu.org/onlinedocs/cpp/Predefined-Macros.html http://docs.sun.com/app/docs/doc/819-5267/6n7c46edt?a=view#bkaoe
Comment 17•15 years ago
|
||
The only difference between our mkdepend and the config/mkdepend in mozilla-central is blassey's change to ifparser.c: bug 463561 - mkdepend crashes while compiling freetype r=bsmedberg a191=beltzner http://hg.mozilla.org/mozilla-central/log/47b4599e95f0/config/mkdepend/ifparser.c I also noticed the variable name 'emxinc' in the part of main.c that we modified. Since EMX means the version of GCC for OS/2, the name 'emxinc' is no longer appropriate now that we also use it for MSVC. So I renamed it 'includeenv', for "the INCLUDE environment variable".
Attachment #362411 -
Flags: review?(nelson)
Comment 18•15 years ago
|
||
Comment on attachment 362411 [details] [diff] [review] Carry over ifparser.c change from mozilla-central, and rename emxinc in main.c If you think 'includeenv' is a little hard to read, 'includepath' would be another good name for that variable.
Assignee | ||
Comment 19•15 years ago
|
||
Comment on attachment 362411 [details] [diff] [review] Carry over ifparser.c change from mozilla-central, and rename emxinc in main.c Wan-Teh, I believe that, in general, we want mkdepend to "compiler" the very same lines of code that the actual compiler will compile, with the same ifdefed code included or excluded. This is especially critical when ifdefs control the #include statements. We have lots of code in NSS that attempts to #include files based on compiler-defined symbols, and was not #including the right files until I made this change. I believe it is desirable to try to be all-inclusive of the compiler's built-in symbols, so as to avoid surprises at some later time when some new code uses some pre-defined symbol previously unused. So, when I found MS's official list of pre-defined symbols, I used it. As for whether we need to do more for gcc or other compilers, maybe. But AFAIK, mkdepend is already considered to be working properly on other platforms. My goal for THIS bug was merely to get it working on Windows. If it turns out not to work on some other platform, then we should fix symbols on those in another bug. BTW, as you would expect, mkdepend adds dependencies of .o's on .c's and .h's, but does not add dependencies for .a/.lib files, nor for .dll/.so files, etc. So, I found that in some cases, when I modified a file, the .o would get rebuilt, but the .so/.lib that incorporates that .o did not get rebuilt. So, we have some more work to do to fix dependencies in NSS, but that should be the subject of another bug. Kai, do you have some way to test this with FF builds? I seem to recall that you implemented some feature that made all of NSS seem to depend on some pseudo-header. Is it time to undo that? Do we need to make the "depend" Makefile target be invoked by (be a dependency of) some other target?
Attachment #362411 -
Flags: review?(nelson) → review+
Comment 20•15 years ago
|
||
We also need to undo the old workaround in PSM's top-level makefile. At http://mxr.mozilla.org/mozilla-central/source/security/manager/Makefile.in#420, we need to pass the 'depend' target on to NSS: 420 clean clobber clobber_all realclean distclean depend:: 421 $(MAKE) -C boot $@ 422 $(MAKE) -C ssl $@ 423 $(MAKE) -C locales $@ 424 ifdef MOZ_XUL 425 $(MAKE) -C pki $@ 426 endif 427 ifndef MOZ_NATIVE_NSS 428 $(MAKE) -C $(topsrcdir)/security/coreconf $(DEFAULT_GMAKE_FLAGS) clean 429 $(MAKE) -C $(topsrcdir)/security/nss/lib $(DEFAULT_GMAKE_FLAGS) clean 430 ifndef SKIP_CHK 431 $(MAKE) -C $(topsrcdir)/security/nss/cmd/lib $(DEFAULT_GMAKE_FLAGS) clean 432 $(MAKE) -C $(topsrcdir)/security/nss/cmd/shlibsign $(DEFAULT_GMAKE_FLAGS) clean 433 endif 434 endif Right now 'depend' is changed to 'clean' before being passed on to NSS.
Comment 21•15 years ago
|
||
I checked in this patch on the NSS trunk (NSS 3.12.3). (I decided to rename the 'emxinc' variable 'includepath'.) Checking in ifparser.c; /cvsroot/mozilla/security/coreconf/mkdepend/ifparser.c,v <-- ifparser.c new revision: 1.3; previous revision: 1.2 done Checking in main.c; /cvsroot/mozilla/security/coreconf/mkdepend/main.c,v <-- main.c new revision: 1.4; previous revision: 1.3 done
Attachment #362411 -
Attachment is obsolete: true
Assignee | ||
Updated•15 years ago
|
Attachment #361923 -
Attachment is obsolete: true
Assignee | ||
Updated•15 years ago
|
Attachment #362341 -
Attachment description: patch as committed → patch as checked in
Comment 22•15 years ago
|
||
Once you declare this patch as fixed, we should undo bug 474473. We need a PSM bug filed for the PSM change.
Assignee | ||
Comment 23•15 years ago
|
||
The good news is that the mkdepend program is now in NSS, can be built on all supported platforms, and is thought to work on all supported platforms. I'm not sure whether it works in cross-platform development, such as building for Windows on a Linux box. If you gmake the "depend" target in mozilla/security/nss, it will build mkdepend and then run it over the tree. Presently, it is necessary to explicitly build the "depend" target, and it must be done after the export target, e.g. gmake export depend libs The bad news is that, on at least one supported platform (Windows, and maybe others) after you've made the depend target, the presence of the file of generated dependencies somehow causes the default target to change from "all" to the first file in the generated dependency file. :( Also, the generated dependencies includes the .o/.obj files that must be build from .c files, but does not include dependencies of libraries on object files or other libraries. That info must still be manually entered in NSS Makefiles. So, there's more makefile work to be done before this bug is resolved/fixed.
Comment 24•3 years ago
|
||
This was changed by https://bugzilla.mozilla.org/show_bug.cgi?id=1438431
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•