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

dev-ruby/facter: ebuild cleanup #9339

Closed
wants to merge 1 commit into from

Conversation

puleglot
Copy link
Contributor

Cleanup:

  • do not use ${S} set by ruby-ng.eclass
  • correctly generate facter.rb, FACTERDIR environment variable is no
    longer required
  • remove no longer needed environment file
  • remove no longer needed multilib magic

Bug: https://bugs.gentoo.org/601746

Cleanup:
- do not use ${S} set by ruby-ng.eclass
- correctly generate facter.rb, FACTERDIR environment variable is no
  longer required
- remove no longer needed environment file
- remove no longer needed multilib magic

Bug: https://bugs.gentoo.org/601746
@gentoo-bot
Copy link

Pull Request assignment

Areas affected: ebuilds
Packages affected: dev-ruby/facter

dev-ruby/facter: @prometheanfire, @gentoo/sysadmin, @gentoo/ruby

Bugs linked: 601746

In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

@gentoo-bot gentoo-bot added assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Jul 24, 2018
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2018-07-24 23:55 UTC
Newest commit scanned: 9508da0
Status: ✅ good

No issues found

Copy link
Member

@prometheanfire prometheanfire left a comment

Choose a reason for hiding this comment

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

I'll need to test this (or better yet have @graaff check it since I use puppet-agent).

ruby-ng_pkg_setup
}

src_unpack() {
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 this section the default already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to avoid ruby-ng_src_unpack()

# restore ${S} and override all phases exported by ruby-ng.eclass
S="${WORKDIR}/${P}"

pkg_setup() {
Copy link
Member

Choose a reason for hiding this comment

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

ruby isn't actually needed for this package to work, so no need to run it's pkg_setup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This package already depends on ruby via ruby-ng.eclass (RUBY_OPTIONAL != yes). It can be made optional, but this should be done via use flag. Otherwise we can't properly install facter.rb for all ruby targets.

Copy link
Member

Choose a reason for hiding this comment

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

ya, should probably be made optional then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't expect me to do that in this PR as it probably require changes in reverse deps (app-admin/puppet). :)

Copy link
Member

Choose a reason for hiding this comment

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

ok, but you can do multiple commits in a PR, but that's up to you :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, build system does not allow to disable ruby support. So this needs either additional hacks in the ebuild or patches for build system.

local mycmakeargs=(
-DCMAKE_VERBOSE_MAKEFILE=ON
-DCMAKE_BUILD_TYPE=None
Copy link
Member

Choose a reason for hiding this comment

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

why'd you get rid of the build type? and install prefix? and blkid location?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CMAKE_BUILD_TYPE and CMAKE_INSTALL_PREFIX are passed by cmake-utils.eclass. Do we really need to override them? BLKID_LIBRARY is not used by build system for a long time:
puppetlabs/facter@364b947#diff-dd2044c7974dda8071b268f0652e866e

@graaff
Copy link
Member

graaff commented Jul 25, 2018

No comments from the ruby team.

@puleglot puleglot deleted the facter-cleanup branch July 26, 2018 08:15
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). bug linked Bug/Closes found in footer, and cross-linked with the PR.
Projects
None yet
5 participants