I think it would be good to do what you propose, eventually. Emphasis on the eventually, unless there's a burning need for it on the part of a contributor.<br><br>However, the KWSys files are not directly push-able into our git repository: we have a server-side hook that rejects pushes containing changes to KWSys files. This makes accepting patches for it a bit more work for us than a simple "git am -3" -- the real KWSys files are still in a shared CVS repository, and we mirror them into our git repo through a robot. So... we have to apply your patch to the CVS repo, and wait for the mirror to replay them into the CMake git repo.<br>
<br>Furthermore, you cannot simply analyze CMake and say that some KWSys functionality is definitively "unused" -- KWSys is shared among many, many projects, (including VTK, ITK, gccxml, ParaView, and others...) any of which may actually be using something in it that you propose to remove.<br>
<br>Personally, I hate much of the code that is in SystemInformation.cxx, and I try to avoid using it or relying on it for anything critically cross-platform. As you've observed, there are several holes in it that are in need of plugging to make it robust and useful/use-able...<br>
<br>Looks like your patch was based on some other commit that is not in 'master' ...? I get this when I try to apply it:<br><br>davidcole@HUT11 ~/Dashboards/My Tests/CMake (sysinfo-rework)<br>$ git am -3 ~/Downloads/0001-remove-trailing-whitespace.patch<br>
Applying: remove trailing whitespace<br>fatal: sha1 information is lacking or useless (Source/kwsys/SystemInformation.cxx).<br>Repository lacks necessary blobs to fall back on 3-way merge.<br>Cannot fall back to three-way merge.<br>
Patch failed at 0001 remove trailing whitespace<br>When you have resolved this problem run "git am --resolved".<br>If you would prefer to skip this patch, instead run "git am --skip".<br>To restore the original branch and stop patching run "git am --abort".<br>
<br><br>I'm willing to remove things that are truly unused, and cluttering up the interface. But first we have to be absolutely sure they are unused across all KWSys clients. And I'm more inclined to add to the interface to return a string for a CPU identifier than to *change* the existing interface. I'd rather leave well enough alone as much as possible in KWSys, and add new stuff that is fully cross-platform as needed moving forward.<br>
<br><br>HTH,<br>David<br><br><br><div class="gmail_quote">On Sat, Nov 6, 2010 at 3:31 PM, Rolf Eike Beer <span dir="ltr"><<a href="mailto:eike@sf-mail.de">eike@sf-mail.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
I wanted to have a look if I could fix one of my bugs (0010895: Processor<br>
detection is very x86-specific). What I found was some things in kwsys classes<br>
I'm not sure about.<br>
<br>
First there is a bunch of files that have trailing whitespaces over and over<br>
which seems rather randomly scattered over the files. I did<br>
<br>
 for i in *; do sed 's/[ \t]*$//' -i $i; done<br>
<br>
which lead to the attached patch (0001).<br>
<br>
Next I found a thing called testSystemInformation.cxx. Looks like it is a<br>
program to debug the SystemInformation class. It is the only user of some<br>
things like DoesCPUSupportCPUID(), GetProcessorAPICID(), and<br>
DoesCPUSupportFeature() from that class. What about just kicking them out?<br>
AFAICT this is Windows-only stuff so this is pretty useless on nearly all<br>
architectures anyway. I just deleted some of them and it still compiles<br>
without problems so I don't see any problems (patch 0002).<br>
<br>
The next thing is the root cause of the above bug: nearly everything in that<br>
class is centered around Intel like CPUs. While that is indeed the majority of<br>
the platforms that use CMake (at least in numbers of users) this makes support<br>
for other CPUs a bit harder. E.g. storing the processor family as integer<br>
makes sense on x86 as it's encoded this way there but what /proc/cpuinfo on my<br>
C3600 prints as family id is "PA-RISC 2.0" which doesn't fit in an integer at<br>
all. I would like to rework that so the transition from integer to string is<br>
done immediately on query and the string is stored also on x86 so non-x86<br>
would work without any further action at least on Linux.<br>
<br>
Comments?<br>
<font color="#888888"><br>
Eike<br>
</font><br>--<br>
<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Please keep messages on-topic and check the CMake FAQ at: <a href="http://www.cmake.org/Wiki/CMake_FAQ" target="_blank">http://www.cmake.org/Wiki/CMake_FAQ</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers" target="_blank">http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers</a><br></blockquote></div><br>