Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.
Question about the intentions for makefiles.
In the crt Makefile.am, there is a note before the lib64/crtdll.a line regarding the reason that SED is being used instead of $(subst …) variables is that the $(subst …) variable is a GNU extension. However, there are %-style pattern substitution rules, which is also a gnu extension, isn't it? Which is intended? (Or are you allowing %-style patterns?)
The reason I ask is I'm looking at doing a patch for 32/64 bit splitting of the DTDEF and DTLIB commands, and can clean up a couple things at the same time if GNU %-style pattern defs are allowed…
(The real problem is that the "foreign" automake type turns off portability warnings, which means that if you want them, you need to add the -Wportability to the AM_INIT_AUTOMAKE of configure.ac, and there's a bug in automake that basically ignores the command line option -Wall for the foreign style)
I'd like to keep deviations with GNU-only make things to a minimum. In some cases, though, it's just too annoying to do it portably. Can you give a rough idea of what you propose?
I'll put it in the patch area for review but the GNU specific thing has to do with (note that there are more things in the patch..which basically take the -mi386 -as-flags=-32 into a 32bit or 64bit specific DTLIB command line define) (note, pretend I'm talking about 64 bit at same time, I just grabbed the 32 bit diff part)
-lib32/libcrtdll.a lib32/libmsvcrt.a lib32/libmsvcr80.a:
- $(DTLIB) -dllname `echo $@ | $(SED) 's|lib32/lib||;s|\.a|.dll|'` -m i386 -as-flags=-32
+lib32/libcrtdll.a lib32/libmsvcrt.a lib32/libmsvcr80.a: lib32/lib%.a : lib32/%.def
+ $(DTDEF32) $< -dllname $*.dll
- $(DTDEF) $(top_srcdir)/lib32/$*.def -m i386 -as-flags=-32
+ $(DTDEF32) $<
Note the pattern rules now use the stems for dependencies and command line. Makes it print more nicely, adds the dependencies properly, and you can see what's going on. In addition, the DTLIB style command line (with the sed part) would only be used in the overrides of the AR commands (the ones that have both a .def and source files).
The other note is that I don't like the special handling of the crtdll, msvcrt, and msvcr80. I would propose one of two things. First would be add the LIBRARY line in the actual .def files so that you don't need the special handling, since the only difference in building you have is the -dllname (I understand that that may not be feasible). If you do this, you would probably be a little more compatible (because I think the TARGETS: TARGET-PATTERN: PREREQ-LIST static pattern rule is less portable than just the plain TARGET-PATTERN: DEPENDENCY-PATTERN style)
Or, if not feasible to modify the .def files, what I would do, is split out the special libraries from the main set:
lib32defonlylibs = lib32/libyada.a ….
lib32needdllnamelibs = lib32/libcrtdll.a lib32/libmsvcrt.a
lib32_DATA = $(lib32defonlylibs) $(lib32needdllnamelibs)
$(lib32defonlylibs): lib32/lib%.a : lib32/%.def
$(lib32needdllnamelibs): lib32/lib%.a : lib32/%.def
$(DTDEF32) $< -dllname $*
Doing the full patch brought up a couple of TODO questions that I had (that should be documented if they are on purpose):
Cleanup the temp = dxerr.c, test.c define.
Examine whether the exclusion of AM_CPPFLAGS in the builds of the combined def/source libraries is appropriate.
Examine whether the inclusion of AM_CPPFLAGS (specifically the -D_CRTBLD flag) is appropriate for the test programs.
Examine whether the exclusion of AM_CFLAGS is appropriate for the libscrnsavw.a build set (since the -DUNICODE overrides them)
Examine whether the exclusion of AM_CFLAGS in the test case override is apporpriate (it may be for t_intrinsic because of the modification of the -std= portion)
Examine whether it would be a good idea to add the appropriate .def file as a dependency for each of the combined def/source libraries (e.g: lib64_libshell32_a_DEPENDENCIES = lib32/shell32.def)
Finally, examine whether it would be better to get rid of the DTLIB command line explicitly specify the .def file in the AR command line (e.g.: lib64_libshell32_a_AR = $(DTDEF64) $(top_srcdir)/lib64/shell32.def; $(AR) $(ARFLAGS) -- you can then remove the DTLIB command definitions (just a cleanup), and output is prettier, and it makes it more obvious the dependency.
As I said, full patch will be posted in a few minutes to review (without any of the TODO items done).
Thank you for being so thorough! I will do likewise with replying.
First, regarding _DEPENDENCIES, that is not an appropriate place for def files, as a def is essentially a source file in concept. We can list it in the _SOURCES vars, much like you can list a header in such a place. This would be for readability, mostly.
For removing DTLIB and specifying the def file directly, we can of course do it. What is the proposed gain? I was trying to factor out code.
Including AM_*FLAGS in the override sections I generally do because when you have a local *FLAG, those are not included by default. I will examine each specific case you mention.
Regarding the choice of _DEPENDENCIES for the extra def files. Unfortunately, automake doesn't convey the dependency information up the chain unless there are implicit build rules for the .def files. Since this isn't possible with the way that these archives are created, it means that the actual dependency on the .def file isn't conveyed to the final library build. Since you cannot put the .def file in the src_lib* variable definition (since each arch has its own def file), the only way to convey the dependency is to place the file in the _DEPENDENCIES. (Try declaring say lib64_libshell32_a_SOURCES to include lib64/shell32.def and you will see that in the final Makefile.in, the .def file is never used as a dependency in the final library target).
For removing the DTLIB command line: Personal opinion more than anything. First, it took me a good half-hour looking at it figuring out that the only thing DTLIB lib was take the target and convert to a def file (since it isn't documented :)). The second thing is that by placing the .def file directly on the _AR redefine, it makes it easier for someone to see right away what's going on, rather than wading through two different defs. The third thing is that with the (proposed) arch split, it removes a level of indirection from each arch. Finally, it removes the SED comand from the make output: which means instead of seeing:
dlltool ….. /opt/src/repos/mingw-w64-svn.git/mingw-w64-crt/`echo lib64/libksuser.a | /bin/sed 's|/lib|/|;s|\.a|.def|'`
dlltool …. /opt/src/repos/mingw-w64-svn.git/mingw-w64-crt/lib64/libkuser.def
I know exactly what the final command was. I realize it may be minor but it saved me a little time finding a typo.
Regarding the AM*_FLAGS - I usually do the same, which means that it was surprising to me to see that including the AM*_FLAGS in the overrides was the exception, and not the rule, in the Makefile. Which is why I suggested examining them, and I think documenting *WHY* they aren't being used would be a good idea (if they conflict or something).
I incorporated one part of your suggestion in r2160, adding the proper dependency to the generic rule. Looking at the rest now.
See r2161 for changes using the target-pattern thingie.