[Driver] Disable OpenSUSE rules for OpenSUSE/SLES 10 and older

[mgorny]

Summary

Disable the OpenSUSE rules for OpenSUSE versions older than 11 as they
are incompatible with the old binutils on that distribution.

Differential Revision: https://reviews.llvm.org/D24954

[ismail]

This will break SLES11 and later so it's not OK. Also note that SLES10 is not supported anymore.

[mgorny]

Does this one look better for you? Yes, I know it's unsupported, that's why I don't really want to put any more effort on it than it is absolutely necessary to make clang work somehow.

[mgorny]

@ismail, ping.

[ismail]

That looks good to me but I am not a reviewer. Someone else must approve.

[bruno]

lib/Driver/ToolChains.cpp

3915–3919

You should keep using the VFS to obtain the file. You probably also want to add a comment here to describe what you mentioned in the patch Summary.

[mgorny]

lib/Driver/ToolChains.cpp

3915–3919

Hmm, this method is consistent with all the other distributions in the method. It seems that getVFS() is only used for exists() checks there. Are you sure I should change this, without touching the other reads first?

[bruno]

lib/Driver/ToolChains.cpp

3915–3919

Right, there seems to be some inconsistent usage here. Ideally we should go through the VFS, but given the precedence it's understandable if you don't make it in this patch.

AFAIU, this drops support for SLES10 and I guess that this would also be true for SLES9? It seems that the checking could be improved a bit by checking the digits (as in the distros above)? Also add a comment explaining why.

[mgorny]

I'm going to delay this one a bit. I've already fixed all other distro checks to use VFS. Now I'd like to update them to use proper numeric parsing.

[mgorny]

I think I've addressed all your concerns now.

[bruno]

lib/Driver/ToolChains.cpp

3918

This file usually has 5-6 lines, can you use 8 instead?

3921

You probably want to explicitly skip the lines you're not interested to make it a bit more clear. AFAIU, the VERSION is usually on the second line. How about:

if (Line.first.trim() != "VERSION")
  continue;
<... then split and parse the value>

[mgorny]

Refactored the code as requested. Also I've noticed that if VERSION had invalid value, the code continued reading the file — now it immediately returns UnknownDistro in that case.

[bruno]

lib/Driver/ToolChains.cpp

3923

You can check Line.trim() != VERSION before the split as to do not try to split unnecessary lines.

[mgorny]

lib/Driver/ToolChains.cpp

3923

You mean line.trim().startswith(), correct?

[bruno]

lib/Driver/ToolChains.cpp

3923

Yep, you got the idea :-)

[mgorny]

Updated to perform .startswith() check before splitting.

[bruno]

LGTM!

lib/Driver/ToolChains.cpp

else if (Data.startswith("wheezy/sid") || Data[0] == '7')
       return DebianWheezy;
     else if (Data.startswith("jessie/sid") || Data[0] == '8')
       return DebianJessie;
     else if (Data.startswith("stretch/sid") || Data[0] == '9')
       return DebianStretch;
     return UnknownDistro;
   }
 
-  if (D.getVFS().exists("/etc/SuSE-release"))
-    return OpenSUSE;
+  File = llvm::MemoryBuffer::getFile("/etc/SuSE-release");
+  if (File)
+    return llvm::StringSwitch<Distro>(File.get()->getBuffer())
+      .StartsWith("openSUSE", OpenSUSE)
+      .Default(UnknownDistro);
 
   if (D.getVFS().exists("/etc/exherbo-release"))
     return Exherbo;
 
   if (D.getVFS().exists("/etc/arch-release"))
     return ArchLinux;
 
   return UnknownDistro;
 }
 
 /// \brief Get our best guess at the multiarch triple for a target.
 ///