-
Notifications
You must be signed in to change notification settings - Fork 162
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
Rewrite Mainframer in Rust. #191
Changes from 15 commits
16d3165
fff43b9
e718688
d4a6f85
aff1522
c5d3760
412eb38
6e287bd
12550ec
5f109d2
601297a
b836e2d
0e212a6
3bef0c7
a49634f
d06f667
26c2b41
907efe5
c333da1
07c1c7e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,5 +5,8 @@ | |
.DS_Store | ||
build/ | ||
|
||
# Rust | ||
target | ||
|
||
# Share code style. | ||
!.idea/codeStyleSettings.xml |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
[package] | ||
name = "mainframer" | ||
version = "2.1.0" | ||
authors = ["Artem Zinnatullin <[email protected]>", "Mainframer Developers and Contributors"] | ||
|
||
[dependencies] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,9 +20,6 @@ if [ "$USER_ID" == "0" ]; then | |
echo "Warning: running as r00t." | ||
fi | ||
|
||
# Run shellcheck. | ||
docker run --rm --env SHELLCHECK_OPTS="--exclude SC2088" --volume `"pwd"`:/scripts:ro koalaman/shellcheck:v0.4.6 /scripts/mainframer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank God. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, actually I'm planning to add it back later to check our integration tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created #194 to track that |
||
|
||
docker build -t mainframer:latest . | ||
|
||
# Command will run inside a container. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,15 +3,15 @@ FROM ubuntu:16.04 | |
MAINTAINER Mainframer Team | ||
|
||
# "sudo": switch user in entrypoint. | ||
# "curl rustup": build Mainframer and sample Rust project. | ||
# "openssh-server": testing. | ||
# "openjdk-8-jdk": build sample Gradle, Maven, Buck projects. | ||
# "golang": build sample Go project. | ||
# "clang": build sample Clang project. | ||
# "build-essential": build sample GCC project. | ||
# "lib32stdc++6 lib32z1 unzip": build sample Android project. | ||
# "ant python git": build sample Buck project. | ||
# "curl rustup": build sample Rust project. | ||
RUN apt-get update --quiet && \ | ||
RUN apt-get update --quiet > /dev/null && \ | ||
apt-get --assume-yes --quiet install sudo openssh-server openjdk-8-jdk \ | ||
golang clang build-essential lib32stdc++6 lib32z1 unzip ant python git curl && \ | ||
curl -sf -L https://static.rust-lang.org/rustup.sh | sh | ||
|
@@ -22,14 +22,18 @@ ENV ANDROID_HOME /opt/android-sdk-linux | |
ENV PATH ${PATH}:${ANDROID_HOME}/tools:${ANDROID_HOME}/tools/bin:${ANDROID_HOME}/platform-tools | ||
ENV ANDROID_SDK_INSTALL_COMPONENT "echo \"y\" | \"$ANDROID_HOME\"/tools/bin/sdkmanager --verbose" | ||
|
||
# Give all users access to Android SDK (otherwise build_user won't be able to access it). | ||
RUN mkdir -p $ANDROID_HOME && \ | ||
curl https://dl.google.com/android/repository/$ANDROID_SDK_FILE_NAME --progress-bar --location --output $ANDROID_SDK_FILE_NAME && \ | ||
unzip $ANDROID_SDK_FILE_NAME -d $ANDROID_HOME && \ | ||
echo "Unzipping Android SDK" && \ | ||
unzip -qq $ANDROID_SDK_FILE_NAME -d $ANDROID_HOME && \ | ||
rm $ANDROID_SDK_FILE_NAME && \ | ||
eval $ANDROID_SDK_INSTALL_COMPONENT '"tools"' && \ | ||
eval $ANDROID_SDK_INSTALL_COMPONENT '"platform-tools"' && \ | ||
eval $ANDROID_SDK_INSTALL_COMPONENT '"build-tools;25.0.2"' && \ | ||
eval $ANDROID_SDK_INSTALL_COMPONENT '"platforms;android-25"' | ||
echo "Installing Android SDK components" && \ | ||
eval $ANDROID_SDK_INSTALL_COMPONENT '"tools"' > /dev/null && \ | ||
eval $ANDROID_SDK_INSTALL_COMPONENT '"platform-tools"' > /dev/null && \ | ||
eval $ANDROID_SDK_INSTALL_COMPONENT '"build-tools;25.0.2"' > /dev/null && \ | ||
eval $ANDROID_SDK_INSTALL_COMPONENT '"platforms;android-25"' > /dev/null && \ | ||
chmod -R 777 "$ANDROID_HOME" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved it here, previously we gave ownership of this folder to the build user in entrypoint script which was slowing down iterative testing as it was running every time on container startup There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 777, ruly? Chmod777:
|
||
|
||
# Entrypoint script will allow us run as non-root in the container. | ||
COPY ci/docker/entrypoint.sh /usr/local/bin/entrypoint.sh | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
#[derive(Debug, PartialEq, Eq)] | ||
pub struct Args { | ||
pub command: String | ||
} | ||
|
||
impl Args { | ||
pub fn parse(raw_args: Vec<String>) -> Result<Args, String> { | ||
match raw_args.len() { | ||
0 => Err(String::from("Please pass remote command.")), // TODO more user friendly message, for now it's consistent with Bash version. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh I see the pattern here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still like the String docs use But There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not aware of any convention here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Damn Rust, String is most confusing type so far, so many options jeez (and I haven't really used Let's stick to |
||
_ => Ok(Args { | ||
command: { | ||
let str: String = raw_args | ||
.iter() | ||
.cloned() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because you own There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice! didn't know about this option, thx! |
||
// Add space between parameters. | ||
.map(|arg| format!("{} ", arg)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess, this isn't important, but this code will allocate a new String for result. This is sorta wasteful given that you already have a owned There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heh, I would thought that Compiler will be able to optimize it here |
||
.collect(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ... but better, I guess you are looking for something like Kotlin's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. holy moly, this is now legit oneliner, THX! |
||
|
||
String::from(str.trim()) | ||
} | ||
}) | ||
} | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
|
||
#[test] | ||
fn parse_command_passed_as_single_parameter() { | ||
let raw_args = vec![String::from("test command")]; | ||
assert_eq!(Args::parse(raw_args), Ok(Args { command: String::from("test command") })); | ||
} | ||
|
||
#[test] | ||
fn parse_empty() { | ||
let raw_args: Vec<String> = vec![]; | ||
assert_eq!(Args::parse(raw_args), Err(String::from("Please pass remote command."))); | ||
} | ||
|
||
#[test] | ||
fn parse_command_passed_as_multiple_parameters() { | ||
let raw_args = vec![String::from("test"), String::from("command")]; | ||
assert_eq!(Args::parse(raw_args), Ok(Args { command: String::from("test command") })); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In future I'd like to detach version from source code and get it from current git tag, but we didn't do it with Bash so for simplicity I didn't do it in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to open an issue about it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeeep, here you go #192