Hello Charles, * Charles Wilson wrote on Wed, Jan 21, 2009 at 04:21:01PM CET: > Thanks, Peter; your validation on msvc is useful and encouraging. Now > I'm just waiting for a go/no-go from one of the four libtool > maintainers: Gary Vaughan, Peter O'Gorman, Ralf Wildenhues, or Bob > Friesenhahn.
I am very sorry that reviewing takes so long. Mostly this is due to time constraints on my side. You can make things easier by splitting them into logically independent (hopefully small) pieces. I acknowledge that some of your other patches may not be splittable further. This particular patch does two logically separate things: 1) repair the creation of the splitted wrapper script. 2) reorganize portability macros inside the wrapper C code. Part (1) is easy to review: it is obvious that regressions are very unlikely to be system-dependent. One does get the impression that it might just be more efficient to let libtool save the cwrapper text somewhere and the program just cat that. But still, this part is ok, please apply. Part (2) is a bit unobvious when merely looking at the diff. The reorganized lines look ok, but not like a clear improvement from the earlier one: both are not fully logical. And then, why is realpath only declared if __CYGWIN__ and __STRICT_ANSI__ && !defined _MSC_VER before the patch, but __CYGWIN__ and __STRICT_ANSI__ && !defined __MINGW32__ after the patch? FWIW, realpath is used only if !defined S_ISLNK. This makes me wince (no pun intended), thinking that just maybe not all systems this is relevant have been duly tested (what about MinGW plus -std=c89?). Why is this patch not accompanied by a testsuite addition using -std=c89 -Werror on a program that creates a C wrapper? Thanks for all your work on this. Cheers, Ralf
