Skip to content

Improve build time of x86_64-linux-gnu target #1760

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Improve build time of x86_64-linux-gnu target #1760

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 2 additions & 5 deletions ci/docker/x86_64-unknown-linux-gnu/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
# Source from https://github.com/rust-lang-deprecated/rust-buildbot/blob/master/slaves/dist/Dockerfile
FROM alexcrichton/rust-slave-dist:2015-10-20b
USER root

WORKDIR /
RUN curl https://www.cpan.org/src/5.0/perl-5.28.0.tar.gz | tar xzf -
WORKDIR /perl-5.28.0
RUN ./configure.gnu
RUN make -j$(nproc)
RUN make install
RUN curl -L https://github.com/lzutao/rustup.rs/releases/download/perl-5.28.0/perl-5.28.0.tar.gz | tar xzf - -C /
Copy link
Contributor Author

@tesuji tesuji Apr 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

We should upload it to somewhere rather than in my own fork.

Copy link
Contributor

Choose a reason for hiding this comment

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

Certainly I wouldn't like this part of the change. I do feel like we can find a better solution where we don't use non-packaged perl at all, but I don't see what it is yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's better but not the best solution we have. Should we ping alexcrichton
to see what he think about this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah... @alexcrichton If you have a moment could you weigh in on a better way to do this part of rustup CI? I really don't like storing built binaries of Perl like this, but I also dislike that we end up rebuilding perl an awful lot.

Copy link
Member

Choose a reason for hiding this comment

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

Instead of a prebuilt perl, could this perhaps take the same strategy as #1724 and use the prebuilt images from rust-lang/rust for deployment?

Copy link
Contributor Author

@tesuji tesuji Apr 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I did the change in my branch.
The Travis build log here.

There are two blocking points:

  • The build is failed, but I don't see why it's failed. It complains about
    linker `cc` not found, but gcc and g++ is included in rust
    x86_64 and i686 docker image?
  • The build time is increase. The average build time is 17 minutes and 30
    seconds (with build succeed and all tests passed).
    Now the build time is at least 18 minutes (with tests uncompleted).

Copy link
Contributor

Choose a reason for hiding this comment

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

It's possible that travis will cache the image now for us, reducing the cost of using a larger docker image, but the cc not found is a pain.

Copy link
Contributor Author

@tesuji tesuji Apr 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@kinnison We use rust's images. Those images could be changed or updated so the
cache will be invalid (correct me if I'm wrong).

Copy link
Contributor

Choose a reason for hiding this comment

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

It would, but not very often. (I think at least every 90 days but likely more often than that) so I think it won't be too bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot. We download the tar-compressed docker images from aws with curl (about 1GB). So I don't think Travis would (automatically) cache this file for us. We could force Travis cache this file, but it's not recommended by Travis and is inefficient.