Thanks for your input and taking an interest in the specifics Corinna.
Coincidentally upstream just released 3.1.2, so I may as well build and
upload that version with the patches added back in.

> 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.

Would be nice to learn the details. Maybe we can tweak this in Cygwin
for the future...?

Looking closer at Source/kwsys/SystemInformation.cxx, now I'm not as sure.
Without the patch, the special-case behavior for Cygwin is looking for
"cpu count" in /proc/cpuinfo, which I don't see on my machine. The Linux
code that the patch switches to look for "physical id" and "cpu cores,"
which I also don't see.

The changes to read "MemTotal:" and "MemFree:" from /proc/meminfo,
"VmRSS:" from /proc/self/status, and having GetProcessId() use getpid()
instead of returning -1 should work fine though.

Yes. It's not the task of the application to second-guess the
underlying "OS" (Cygwin taking the role in a way).

There are various wrapper scripts in autotools to do the same thing
(use Cygwin as a build environment for non-Cygwin compilers), they just
tend to make decisions on what kinds of path translation to do based on
`uname` rather than compile-time #ifdef's. Though with CMake, users can
always have the option of using the binary installer downloads and
running those from within Cygwin. So the Cygwin-distributed build of
cmake has a bit more room to follow Cygwin best practices, at the cost
of not supporting as many use cases, and requiring local patches that
need to be kept updated.

Cygwin's access() call is not for POSIX paths only.
It handles native Windows paths as well.

Oh cool, I was not aware of that.

As for FileIsFullPath, what is the change about?

The change disables the following snippet of code for the Cygwin case:

 // On Windows, the name must be at least two characters long.
 if(len < 2)
   {
   return false;
   }
 if(in_name[1] == ':')
   {
   return true;
   }
 if(in_name[0] == '\\')
   {
   return true;
   }

instead using the Unix condition of false if len < 1, true if
in_name[0] == '~' or '/', false otherwise. The (len < 2) part is
obviously wrong for Cygwin, but this function isn't using access() so
the change would make this function no longer return true for C:\ style
Windows paths or UNC paths. A Cygwin application should probably be
given those paths in /cygdrive/c/ form, but I'm not sure of all the
places in cmake's code where this function gets used and what the
consequences are.

-Tony

Reply via email to