[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 ↗

(On Diff #72609)

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 ↗

(On Diff #72609)

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 ↗

(On Diff #72609)

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

3937 ↗

(On Diff #75357)

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

3940 ↗

(On Diff #75357)

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

3942 ↗

(On Diff #75615)

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

[mgorny]

lib/Driver/ToolChains.cpp

3942 ↗

(On Diff #75615)

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

[bruno]

lib/Driver/ToolChains.cpp

3942 ↗

(On Diff #75615)

Yep, you got the idea :-)

[mgorny]

Updated to perform .startswith() check before splitting.

[bruno]

LGTM!

cfe/trunk/lib/Driver/ToolChains.cpp

if (File) {
     return llvm::StringSwitch<Distro>(Data.split("\n").first)
         .Case("squeeze/sid", DebianSqueeze)
         .Case("wheezy/sid", DebianWheezy)
         .Case("jessie/sid", DebianJessie)
         .Case("stretch/sid", DebianStretch)
         .Default(UnknownDistro);
   }
 
-  if (VFS.exists("/etc/SuSE-release"))
+  File = VFS.getBufferForFile("/etc/SuSE-release");
+  if (File) {
+    StringRef Data = File.get()->getBuffer();
+    SmallVector<StringRef, 8> Lines;
+    Data.split(Lines, "\n");
+    for (const StringRef& Line : Lines) {
+      if (!Line.trim().startswith("VERSION"))
+        continue;
+      std::pair<StringRef, StringRef> SplitLine = Line.split('=');
+      int Version;
+      // OpenSUSE/SLES 10 and older are not supported and not compatible
+      // with our rules, so just treat them as UnknownDistro.
+      if (!SplitLine.second.trim().getAsInteger(10, Version) &&
+          Version > 10)
         return OpenSUSE;
+      return UnknownDistro;
+    }
+    return UnknownDistro;
+  }
 
   if (VFS.exists("/etc/exherbo-release"))
     return Exherbo;
 
   if (VFS.exists("/etc/arch-release"))
     return ArchLinux;
 
   return UnknownDistro;