[PATCH] D24954: [ToolChains] Disable OpenSUSE rules for SLES10

[Michał Górny via cfe-commits]

mgorny planned changes to this revision.
mgorny added a comment.

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.


https://reviews.llvm.org/D24954



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24954: [ToolChains] Disable OpenSUSE rules for SLES10

[Bruno Cardoso Lopes via cfe-commits]

bruno added inline comments.



Comment at: lib/Driver/ToolChains.cpp:3915
 
-  if (D.getVFS().exists("/etc/SuSE-release"))
-return OpenSUSE;
+  File = llvm::MemoryBuffer::getFile("/etc/SuSE-release");
+  if (File)

mgorny wrote:
> bruno wrote:
> > 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.
> 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?
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.


https://reviews.llvm.org/D24954



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24954: [ToolChains] Disable OpenSUSE rules for SLES10

[Michał Górny via cfe-commits]

mgorny added inline comments.



Comment at: lib/Driver/ToolChains.cpp:3915
 
-  if (D.getVFS().exists("/etc/SuSE-release"))
-return OpenSUSE;
+  File = llvm::MemoryBuffer::getFile("/etc/SuSE-release");
+  if (File)

bruno wrote:
> 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.
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?


https://reviews.llvm.org/D24954



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24954: [ToolChains] Disable OpenSUSE rules for SLES10

[Bruno Cardoso Lopes via cfe-commits]

bruno added a reviewer: bruno.
bruno added inline comments.



Comment at: lib/Driver/ToolChains.cpp:3915
 
-  if (D.getVFS().exists("/etc/SuSE-release"))
-return OpenSUSE;
+  File = llvm::MemoryBuffer::getFile("/etc/SuSE-release");
+  if (File)

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.


https://reviews.llvm.org/D24954



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24954: [ToolChains] Disable OpenSUSE rules for SLES10

[İsmail Dönmez via cfe-commits]

ismail added a comment.

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


https://reviews.llvm.org/D24954



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24954: [ToolChains] Disable OpenSUSE rules for SLES10

[Michał Górny via cfe-commits]

mgorny retitled this revision from "[ToolChains] Do not assume OpenSUSE for 
other SUSE variants" to "[ToolChains] Disable OpenSUSE rules for SLES10".
mgorny updated the summary for this revision.
mgorny updated this revision to Diff 72609.
mgorny added a comment.

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.


https://reviews.llvm.org/D24954

Files:
  lib/Driver/ToolChains.cpp

Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -3912,8 +3912,11 @@
 return UnknownDistro;
   }
 
-  if (D.getVFS().exists("/etc/SuSE-release"))
-return OpenSUSE;
+  File = llvm::MemoryBuffer::getFile("/etc/SuSE-release");
+  if (File)
+return llvm::StringSwitch(File.get()->getBuffer())
+  .StartsWith("SUSE Linux Enterprise Server 10", UnknownDistro)
+  .Default(OpenSUSE);
 
   if (D.getVFS().exists("/etc/exherbo-release"))
 return Exherbo;


Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -3912,8 +3912,11 @@
 return UnknownDistro;
   }
 
-  if (D.getVFS().exists("/etc/SuSE-release"))
-return OpenSUSE;
+  File = llvm::MemoryBuffer::getFile("/etc/SuSE-release");
+  if (File)
+return llvm::StringSwitch(File.get()->getBuffer())
+  .StartsWith("SUSE Linux Enterprise Server 10", UnknownDistro)
+  .Default(OpenSUSE);
 
   if (D.getVFS().exists("/etc/exherbo-release"))
 return Exherbo;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits