add solaris rustbuild support #39759
Conversation
Add cputype recognition for Solaris Fixes rust-lang#39729
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
Don't need to catch WindowsError. That was very silly of me.
src/bootstrap/bootstrap.py
Outdated
| @@ -315,7 +315,7 @@ def build_triple(self): | |||
| try: | |||
| ostype = subprocess.check_output(['uname', '-s']).strip().decode(default_encoding) | |||
| cputype = subprocess.check_output(['uname', '-m']).strip().decode(default_encoding) | |||
| except (subprocess.CalledProcessError, WindowsError): | |||
| except subprocess.CalledProcessError: | |||
There was a problem hiding this comment.
Perhaps this could remain the same? (or does it break solaris?)
There was a problem hiding this comment.
It can remain the same if you wish, it does not break Solaris (which is how I missed it the first time). Well, it doesn't break it when it works, but it could break if that error case is tripped. Let me check.
There was a problem hiding this comment.
Yes, as I suspected if the execution of 'isainfo' fails then Python will trip over the Exception handler at runtime:
Traceback (most recent call last):
File "./x.py", line 20, in <module>
bootstrap.main()
File "/builds/srwalker/rsdev/rust.git/src/bootstrap/bootstrap.py", line 492, in main
rb.build = rb.build_triple()
File "/builds/srwalker/rsdev/rust.git/src/bootstrap/bootstrap.py", line 358, in build_triple
except (subprocess.CalledProcessError, WindowsError):
NameError: global name 'WindowsError' is not defined
...and annoyingly, despite pydoc's claims otherwise, Python doesn't raise CalledProcessError here; it raises an OSError() (if the command is missing for some reason). My guess is that it will only raise CalledProcessError() if execution of the command returns failure, as opposed to being unable to execute the process at all. So this should actually catch both CalledProcessError and OSError. I will apply that fix and your other requested change and try the full build again.
There was a problem hiding this comment.
Ah yeah so this clause was added in ad88d50 specifically for Windows, so it'd be great to at least have the Windows behavior here stay the same (although I'm not sure how to do that best)
There was a problem hiding this comment.
WindowsError is just a subclass of OSError, so that would be the portable equivalent:
exception WindowsError
Raised when a Windows-specific error occurs or when the error number does not correspond to an errno value. The winerror and strerror values are created from the return values of the GetLastError() and FormatMessage() functions from the Windows Platform API. The errno value maps the winerror value to corresponding errno.h values. This is a subclass of OSError.
So I'll just change it to use that, avoiding the portability issues entirely.
There was a problem hiding this comment.
Also, as of Python 3.3, WindowsError was merged into OSError, so this definitely seems like the right fix:
Changed in version 3.3: EnvironmentError, IOError, WindowsError, socket.error, select.error and mmap.error have been merged into OSError, and the constructor may return a subclass.
src/libstd/build.rs
Outdated
| @@ -46,6 +46,8 @@ fn main() { | |||
| } else if target.contains("dragonfly") || target.contains("bitrig") || | |||
| target.contains("netbsd") || target.contains("openbsd") { | |||
| println!("cargo:rustc-link-lib=pthread"); | |||
| } else if target.contains("solaris") { | |||
| println!("cargo:rustc-link-lib=gcc_s"); | |||
There was a problem hiding this comment.
I believe if this is linked through libunwind then we can avoid linking it here as well
There was a problem hiding this comment.
Ok, I will remove this and try a local build again.
remove unnecessary gcc_s addition in libstd for Solaris
|
Sounds great! @bors: r+ |
|
📌 Commit 3807e1f has been approved by |
|
@bors rollup |
add solaris rustbuild support Add Solaris as recognized ostype Add cputype recognition for Solaris Fixes rust-lang#39729 A future pull request will discriminate between the commercial release and older opensource derivatives to account for divergence, for now, this is compatible with both.
add solaris rustbuild support Add Solaris as recognized ostype Add cputype recognition for Solaris Fixes #39729 A future pull request will discriminate between the commercial release and older opensource derivatives to account for divergence, for now, this is compatible with both.
|
💔 Test failed - status-travis |
|
@bors: retry
* network error
…On Mon, Feb 13, 2017 at 3:04 AM bors ***@***.***> wrote:
💔 Test failed - status-travis
<https://travis-ci.org/rust-lang/rust/builds/201060279>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#39759 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95F0azJriEyy0Oi-WjqZAlhO08oisks5rcByIgaJpZM4L-Y7P>
.
|
add solaris rustbuild support Add Solaris as recognized ostype Add cputype recognition for Solaris Fixes rust-lang#39729 A future pull request will discriminate between the commercial release and older opensource derivatives to account for divergence, for now, this is compatible with both.
Add Solaris as recognized ostype
Add cputype recognition for Solaris
Fixes #39729
A future pull request will discriminate between the commercial release and older opensource derivatives to account for divergence, for now, this is compatible with both.