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

app-emulation/vagrant: bump to 1.9.7, drop old #5353

Closed
wants to merge 2 commits into from

Conversation

hydrapolic
Copy link
Contributor

Package-Manager: Portage-2.3.6, Repoman-2.3.2

@gentoo-repo-qa-bot gentoo-repo-qa-bot added self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). labels Aug 8, 2017
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull Request assignment

Areas affected: ebuilds
Packages affected: app-emulation/vagrant

app-emulation/vagrant: @hydrapolic, @gentoo/proxy-maint

Copy link
Member

@mgorny mgorny left a comment

Choose a reason for hiding this comment

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

Please split adding the new version and removing the old one into separate commits.

# This needs to be set because Bundler includes gem paths
# from RubyGems' Gem.paths.
if [ -z $VAGRANT_HOME ]; then
VAGRANT_HOME=$(eval echo "~/.vagrant.d")
Copy link
Member

Choose a reason for hiding this comment

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

What's this monster? Are you trying to force tilde substitution here? Isn't

VAGRANT_HOME=~/.vagrant.d

much simpler? :-P

export VAGRANT_INSTALLER_VERSION="2"

# Determine the OS that we're on, which is used in some later checks.
# It is very important we do this _before_ setting the PATH below
Copy link
Member

Choose a reason for hiding this comment

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

Where is the PATH set? I feel like this comment is no longer relevant.


# Export the OS as an environmental variable that Vagrant can access
# so that it can behave better.
export VAGRANT_DETECTED_OS="${OS}"
Copy link
Member

Choose a reason for hiding this comment

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

Why not inline the unused ${OS} variable in here?

#
# This needs to be set because Bundler includes gem paths
# from RubyGems' Gem.paths.
if [ -z $VAGRANT_HOME ]; then
Copy link
Member

Choose a reason for hiding this comment

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

It'd be nice to use ${foo} consistently everywhere.

VAGRANT_EXECUTABLE="${VAGRANT_DIR}/bin/vagrant"

# Export the VAGRANT_EXECUTABLE so that pre-rubygems can optimize a bit
export VAGRANT_EXECUTABLE
Copy link
Member

Choose a reason for hiding this comment

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

Squash exporting and setting into a single line.

# Copyright 1999-2017 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2

EAPI="5"
Copy link
Member

Choose a reason for hiding this comment

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

EAPI 6, please.

ruby_add_rdepend "
>=dev-ruby/childprocess-0.6.0
>=dev-ruby/erubis-2.7.0
>=dev-ruby/i18n-0.6.0:* <dev-ruby/i18n-0.8.0:*
Copy link
Member

Choose a reason for hiding this comment

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

This can install two different versions in separate slots. I doubt that's what you want. Since there is no version older than 0.6.0, you can just leave the < part.

>=dev-ruby/net-scp-1.2.0
|| ( >=dev-ruby/rest-client-1.6.0:0 dev-ruby/rest-client:2 )
>=dev-ruby/nokogiri-1.7.1
>=dev-ruby/mime-types-2.6.2:* <dev-ruby/mime-types-3:*
Copy link
Member

Choose a reason for hiding this comment

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

Likewise.

# Vagrant installation directory. This sets up proper environmental variables
# so that everything loads and compiles to proper directories.

VAGRANT_DIR="$( ruby -e 'print Gem::default_path[-1] + "/gems/vagrant-1.9.7"' )"
Copy link
Member

Choose a reason for hiding this comment

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

By the way, are you really going to introduce a new script with different version number for every version? This is a real waste of space. How about using vagrant.in and using sed to substitute the version?

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, good point.

if [ -z ${VAGRANT_HOME} ]; then
VAGRANT_HOME="~/.vagrant.d"
fi
export GEM_HOME="$VAGRANT_HOME/gems"
Copy link
Member

Choose a reason for hiding this comment

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

${} here.

>=dev-ruby/net-ssh-4.1.0:*
>=dev-ruby/net-sftp-2.1
>=dev-ruby/net-scp-1.2.0
|| ( >=dev-ruby/rest-client-1.6.0:0 dev-ruby/rest-client:2 )
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the newest version preferred? In || we tend to put option from most preferred to least preferred.

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, forgot we need to inverse the logic.

Package-Manager: Portage-2.3.7, Repoman-2.3.2
Package-Manager: Portage-2.3.7, Repoman-2.3.2
Copy link
Member

@mgorny mgorny left a comment

Choose a reason for hiding this comment

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

Good enough not to delay it more. For the next update, please replace epatch with eapply, possibly updating the patches to -p1 in the process.

@hydrapolic
Copy link
Contributor Author

Thanks

@hydrapolic hydrapolic deleted the vagrant branch August 17, 2017 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else)
Projects
None yet
3 participants