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

Don't use travis_apt_get_update and invoke apt-get update manually #3142

Closed
Closed
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
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ before_cache:
- docker save dash-builder-$BUILD_TARGET-$TRAVIS_JOB_NUMBER $(docker history -q dash-builder-$BUILD_TARGET-$TRAVIS_JOB_NUMBER | grep -v \<missing\>) | gzip -2 > $HOME/cache/docker/dash-builder-$BUILD_TARGET.tar.gz

before_install:
- travis_retry travis_apt_get_update
- sudo rm -rf "${TRAVIS_ROOT}/var/lib/apt/lists/"*
- travis_retry sudo sudo apt-get update 2>&1
- travis_retry sudo apt-get -yq --no-install-suggests --no-install-recommends install docker-ce realpath
Copy link

Choose a reason for hiding this comment

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

Can't we remove this entirely if all we are installing is docker-ce and we have the services: docker set up above?

(Not sure what realpath is used for.)

Copy link
Author

Choose a reason for hiding this comment

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

realpath must be installed in Trusty as it's not available in the base install. In Bionic it became part of coreutils. I'm investigating an upgade to Bionic atm where I already had to remove the realpath package.

Copy link

Choose a reason for hiding this comment

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

Ok, thanks. That was actually a tangent of my original question tho -- we shouldn't need to do this whole apt-get update / install thing at all since we have the docker service setup, right? Meaning all this would be unnecessary.

Copy link
Author

Choose a reason for hiding this comment

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

If I remember correct, I added this to make sure we're on a recent docker version. With the Bionic upgrade, that probably won't be needed as well.


Comment on lines 116 to 120
Copy link

Choose a reason for hiding this comment

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

My suggestion is to remove this entirely and rename the PR.

Copy link

Choose a reason for hiding this comment

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

Sorry, I can't figure out how to use the GitHub "suggestions" feature.

Copy link
Author

Choose a reason for hiding this comment

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

No need, this whole PR will probably get closed as I'm working on #3143 now

install:
Expand Down