Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect mips CPUs in ./configure #38650

Merged
merged 2 commits into from
Jan 13, 2017
Merged

Detect mips CPUs in ./configure #38650

merged 2 commits into from
Jan 13, 2017

Conversation

infinity0
Copy link
Contributor

This mirrors existing logic already in src/bootstrap/bootstrap.py

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

Copy link
Contributor

@xen0n xen0n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting this! I forgot to touch configure in my MIPS host build PR because the MIPS64 platform I was working on is rustbuild-only. Looks good overall!

@@ -388,6 +388,7 @@ msg "inspecting environment"

CFG_OSTYPE=$(uname -s)
CFG_CPUTYPE=$(uname -m)
ENDIAN=$(printf '\1' | od -dAn)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here od isn't need_cmd'd, thus unsafe to rely on. You could refer to the rustup-init.sh implementation for an alternative that only requires head and tail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

od is part of POSIX (including the -d and -A n options), just as head and tail are. Is that good enough? If not, I can do your suggestion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, but some exotic environments indeed doesn't have od; a quick Googling reveals OpenWRT and Maemo and the such. I don't know how many people are compiling Rust on these platforms though...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to minimize the possible impact of this change, could this move into just the mips block below?

@infinity0
Copy link
Contributor Author

Welcome! In the meantime I spotted some more things which are missing:

  • mk/cfg/mips{,el}-unknown-linux-gnu.mk: doesn't define CFG_RUN_TARG_mips{,el}-unknown-linux-gnu
  • mk/cfg/mips64{,el}-unknown-linux-gnuabi64.mk are empty

I'll fix these as well and add them to this PR.

In the long run, does the rust team have any plans to unify the Makefiles and rustup/rustbuild? Seems like unnecessary work to have to maintain both of these...

@xen0n
Copy link
Contributor

xen0n commented Dec 28, 2016

@infinity0 Please refrain from touching the makefiles, they're slated to be deleted pretty soon!

@infinity0
Copy link
Contributor Author

Ouch, OK. Is there a way to not use Cargo to build rust? That is going to make bootstrapping new architectures much much more difficult.

@xen0n
Copy link
Contributor

xen0n commented Dec 28, 2016

@infinity0 No, bootstrapping will be arguably easier, since you no longer have to deal with multiple different build tools with rustbuild. Look at the way rustc is configure'd for the non-x86 hosts, pretty much all of the work you're expected to do for bootstrapping is now concentrated in the codebase itself. See #38314 for a feel of it; bootstrapping is lighter than ever IMO.

@infinity0
Copy link
Contributor Author

Er, I am doing bootstrapping literally right now, and I can tell you it will be harder. All of the previous tools like make etc are already present for other architectures; cargo is not. Large cyclic dependencies also increase the amount of binary non-source code that needs to be trusted when bootstrapping a given whole distribution. From this perspective, it is better for rust to depend on 100 other build tools (that eventually depend on a small set of cyclic-dependency base build tools) than to be cyclicly dependent with cargo. I can appreciate the code is nicer to review than the previous Makefiles, but it would be nice if this could be done in a way that does not hard-depend on cargo. I made a concrete suggestion in #38652 and we can continue the discussion there.

@sylvestre
Copy link
Contributor

👍 to what @infinity0 just said.

@alexcrichton
Copy link
Member

This PR looks good to me, thanks @infinity0!

This mirrors existing logic already in src/bootstrap/bootstrap.py
@infinity0
Copy link
Contributor Author

OK, I've moved ENDIAN down into the mips-specific block. I also added another commit that was necessary to compile the tests. Due to other problems (#39013, #39014) I couldn't test these all the way through to completion, however I notice that these files:

src/tools/compiletest/src/util.rs
src/test/run-make/atomic-lock-free/Makefile

mention mips but not mips64. Is that intentional or do they also need to be fixed?

@alexcrichton
Copy link
Member

@bors: r+

Thanks! It's ok to not update those unless necessary.

@bors
Copy link
Contributor

bors commented Jan 12, 2017

📌 Commit 246e749 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jan 12, 2017

⌛ Testing commit 246e749 with merge dd8e68d...

bors added a commit that referenced this pull request Jan 12, 2017
Detect mips CPUs in ./configure

This mirrors existing logic already in src/bootstrap/bootstrap.py
@bors
Copy link
Contributor

bors commented Jan 13, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing dd8e68d to master...

@bors bors merged commit 246e749 into rust-lang:master Jan 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants