On Thu, 2015-02-05 at 21:41 -0800, Tony Kelman wrote: > > Do you have any specific questions? > > 2.8.9-perl-libs.patch AIUI changes PERL_POSSIBLE_LIBRARY_NAMES from > being set to "perl{PERL_VERSION_STRING} perl" only when the command > `${PERL_EXECUTABLE} -V:libperl` fails, to appending those values any > time PERL_EXECUTABLE is found. Is that command expected to fail in some > circumstances where one would be building a perl library? Or is the > problem that cmake find_library has trouble with the cyg* library prefix?
IIRC the latter. > 2.8.12-gui-qt4, 2.8.12-opengl, 2.8.12-ruby, and 2.8.12-tclsh look pretty > straightforward and worth upstreaming. Should I assume you have not yet > tried to do so? Correct. > #-cygwin.patch, which I had to rebase a few times to apply to the latest > release, does multiple things (good sign it should have been split into > multiple patches): > > First, it disables case-insensitive matching. I see that there are some > non-default/experimental ways of running Cygwin in case sensitive mode, > but that is a runtime configuration. Most users are probably running with > case-insensitive default NTFS. If this change fixes more issues than it > causes I can take your word on it, but this would be hard to convince > upstream about. Is case sensitive matching really the right build-time > behavior to choose for Cygwin's cmake package? Yes, cmake should leave that to Cygwin to handle. There are (or have been) a few cases where a package bundles a cmake module with the same name as a builtin one but with different case and different contents. With case sensitivity on (as I do), the correct module should be chosen. > Second, it corrects some /proc/cpuinfo, /proc/meminfo, etc special-casing > of Cygwin where using the same code as Linux should work. Makes sense, > I'll split this out so it can be discussed on its own w/upstream. > > Lastly, it's disabling handling of Windows paths, and conversion from > Cygwin paths to Windows paths. It does look like PathCygwinToWin32 could > go away if the only place it's used in FileExists is changed to just use > access(). Upstream might find this and the change to FileIsFullPath > debatable though, since a use case for a decent number of people is > running cmake within Cygwin to drive non-Cygwin compilers. Should we > really ignore this use case? Corinna has already explained this. -- Yaakov