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

Add crate build test for thumb* targets. [IRR-2018-embedded] #53190

Merged
merged 15 commits into from
Aug 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -975,9 +975,19 @@ impl Step for Compiletest {
builder.ensure(compile::Rustc { compiler, target });
}

if builder.no_std(target) != Some(true) {
if builder.no_std(target) == Some(true) {
// the `test` doesn't compile for no-std targets
builder.ensure(compile::Std { compiler, target });
} else {
builder.ensure(compile::Test { compiler, target });
}

if builder.no_std(target) == Some(true) {
Copy link
Member

Choose a reason for hiding this comment

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

Should this block be combined with the block above (line 980)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At first, I've written it in the form the second if is merged with the first if.

Later, I divided them into two ifs with clear intention. They are representing the two different purposes/concerns.

Let me explain the intention(s) below.

The first if

        if builder.no_std(target) == Some(true) {
            // the `test` doesn't compile for no-std targets
            builder.ensure(compile::Std { compiler, target });
        } else {
            builder.ensure(compile::Test { compiler, target });
        }

The first if serves for building compile::Test. Unfortunately, you can't build compile::Test for no_std. In that case, build compile::Std instead. It should be in the form of A or B. Adding unrelated C in one arm is not a good idea.

It's also a verbatim copy of @japaric's code found in dist.rs and supposed to be like a idiom which would be used in various places consistently. In the current bootstrap code, test support for no_std target is quite minimum. To widen the supported coverage for no_std, we might want to grep builder.ensure(compile::Test and replace it with this snippet.

But in future, no_std might get libtest support. At that time the whole if must be reverted back to the original single line:

        builder.ensure(compile::Test { compiler, target });

If you merged two ifs, the whole change would be much less trivial.

The second if

        if builder.no_std(target) == Some(true) {
            // for no_std run-make (e.g. thumb*),
            // we need a host compiler which is called by cargo.
            builder.ensure(compile::Std { compiler, target: compiler.host });
        }

The second if serves for a totally different purpose. For the majority of run-make tests, host compiler is NEVER needed. It is needed because we must run cargo and some dependent crate (e.g. cc) requires host compiler to be successfully built.

// for no_std run-make (e.g. thumb*),
// we need a host compiler which is called by cargo.
builder.ensure(compile::Std { compiler, target: compiler.host });
}

builder.ensure(native::TestHelpers { target });
builder.ensure(RemoteCopyLibs { compiler, target });

Expand Down
33 changes: 33 additions & 0 deletions src/test/run-make/git_clone_sha1.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/bin/bash -x

# Copyright 2018 The Rust Project Developers. See the COPYRIGHT
# file at the top-level directory of this distribution and at
# http://rust-lang.org/COPYRIGHT.
#
# Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
# http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
# <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
# option. This file may not be copied, modified, or distributed
# except according to those terms.

# Usage: $0 project_name url sha1
# Get the crate with the specified sha1.
#
# all arguments are required.
#
# See below link for git usage:
# https://stackoverflow.com/questions/3489173#14091182

# Mandatory arguments:
PROJECT_NAME=$1
URL=$2
SHA1=$3

function err_exit() {
echo "ERROR:" $*
exit 1
}

git clone $URL $PROJECT_NAME || err_exit
cd $PROJECT_NAME || err_exit
git reset --hard $SHA1 || err_exit
38 changes: 38 additions & 0 deletions src/test/run-make/thumb-none-cortex-m/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
-include ../../run-make-fulldeps/tools.mk

# How to run this
# $ ./x.py clean
# $ ./x.py test --target thumbv6m-none-eabi,thumbv7m-none-eabi src/test/run-make

# Supported targets:
Copy link
Member

Choose a reason for hiding this comment

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

🎊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we now build required artifacts in ./x.py test execution.

# - thumbv6m-none-eabi (Bare Cortex-M0, M0+, M1)
# - thumbv7em-none-eabi (Bare Cortex-M4, M7)
# - thumbv7em-none-eabihf (Bare Cortex-M4F, M7F, FPU, hardfloat)
# - thumbv7m-none-eabi (Bare Cortex-M3)

# See https://stackoverflow.com/questions/7656425/makefile-ifeq-logical-or
ifneq (,$(filter $(TARGET),thumbv6m-none-eabi thumbv7em-none-eabi thumbv7em-none-eabihf thumbv7m-none-eabi))

# For cargo setting
RUSTC := $(RUSTC_ORIGINAL)
LD_LIBRARY_PATH := $(HOST_RPATH_DIR)
# We need to be outside of 'src' dir in order to run cargo
WORK_DIR := $(TMPDIR)

HERE := $(shell pwd)

CRATE := cortex-m
CRATE_URL := https://github.com/rust-embedded/cortex-m
CRATE_SHA1 := a448e9156e2cb1e556e5441fd65426952ef4b927 # 0.5.0

all:
env
mkdir -p $(WORK_DIR)
-cd $(WORK_DIR) && rm -rf $(CRATE)
cd $(WORK_DIR) && bash -x $(HERE)/../git_clone_sha1.sh $(CRATE) $(CRATE_URL) $(CRATE_SHA1)
cd $(WORK_DIR) && cd $(CRATE) && $(CARGO) build --target $(TARGET) -v
else

all:

endif