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

Package depends #116

Closed
wants to merge 5 commits into from
Closed

Conversation

boc-tothefuture
Copy link

This PR adds support for explicitly setting package names to be passed to FPM without first trying to install them as either build or runtime dependencies. This is required when you need to specify the architecture of a dependency and your build target is an RPM. Without this PR if you specify the dependency with 'i686' puppet is happy but RPM can't resolve the dependency on install. If you specify the dependency as '(x86-32)' then RPM is happy but puppet fails.

If the PR is accepted, I will update the wiki accordingly.

@bernd
Copy link
Owner

bernd commented Aug 24, 2015

Can you show me an example recipe which shows the problem? Also, does the --no-deps option help in your case?

@bernd bernd self-assigned this Aug 24, 2015
@boc-tothefuture
Copy link
Author

@bernd
Here is a gist showing an example recipe with the problem: https://gist.github.com/brian-oconnell/0d1c722a0e306aad3cc9
Here is a gist showing the commands I run: https://gist.github.com/brian-oconnell/4250b689fefbe95162bf
Here is an output with the error: https://gist.github.com/brian-oconnell/c83037c09fedc1673c76

It fails on the fpm-cook - install-deps command because that tries to install both build and regular dependencies. I need the build dependencies to perform my build.

@boc-tothefuture
Copy link
Author

Another option that might be less invasive would be to make a cli change, either a flag or new command, to only install build dependences.

If you prefer that I can submit a PR.

@bernd
Copy link
Owner

bernd commented Aug 27, 2015

Thanks for the update! I need to find some time the next days to fiddle with this.

@bernd
Copy link
Owner

bernd commented Aug 29, 2015

I thought about this and all the other problems we have with Puppet and dependency installations and decided that it might be better to remove the automatic dependency installation.

I prepared a PR in #122 to remove the feature. There is also a more detailed explanation of the reasons. Please have a look and let me know what you think in the comments of that PR. Thank you!

@boc-tothefuture
Copy link
Author

It seems that PR #122 is heading towards not removing the dependency installation.
On reflection, I am not sure we want to extend the DSL in the fashion I have outlined as I think it would add to user confusion.

I did have one historical design question. Why does fpm-cookery merge the build_depends and depends together during dependency verification for the package phase or even the install-deps phase? I understand why the build deps should be installed, but for the items I have worked on I didn't unerstand why the package deps also installed.. or if I wanted them installed why I wouldn't just set build_deps equal to depends.

So assuming we don't want to extend the DSL as done in this PR and depending on the answer to the above question, I see a couple of paths forward. Let me know your thoughts.

  1. Remove the merging of build_depends and depends during dependency installation and packaging. (This depends on the answer to the above question).
  2. Add a new command to just install build_deps
  3. Automatically convert 'x86-32' dependencies to 'i686' dependencies before passing them on to puppet.

@bernd let me know which you prefer, I would be happy to put together a PR for any of the above or other options that you come up with.

@bernd
Copy link
Owner

bernd commented Aug 31, 2015

It seems that PR #122 is heading towards not removing the dependency installation.

Yes, after the feedback I got on that PR the feature seems valuable for you and others. That was also my hope to get some feedback so I can decide if I merge it or not. (I missed to communicate that...)

Why does fpm-cookery merge the build_depends and depends together during dependency verification for the package phase or even the install-deps phase?

The idea was that depends is for stuff that is needed during runtime (and for building it, like libraries) and build_depends for packages like build tools and -dev|devel packages that is only needed for building the project.

Example:

class Foo < FPM::Cookery::Recipe
  build_depends 'libpng12-dev'
  depends 'libpng12-0'
end

Both, libpng12-0 and libpng12-dev are needed to build the software, that's why both dependecy listst are merged. Only the packages specified in depends will be passed as dependencies to fpm to be recorded in the package.

I agree that this can be confusing, especially because there is no documentation yet.

Regarding the three options you suggested:

  1. This might break lots of existing recipes out there. At least the ones I wrote. 😄
  2. This could be a workaround that does not break any existing recipes. 👍 We should also think about disabling the dependency installation by default. (which might break existing setups again, hmm)
  3. This sounds like too specific for rpm to me, I would like to avoid that.

@glensc
Copy link
Contributor

glensc commented Aug 31, 2015

installing depends and build_depends because you need both, looks like packaging problem. i.e

libpng12-dev package must depend on libpng12-0, so installing libpng12-dev would install libpng12-0 as well. you are sure debian -dev packages don't pull in appropriate runtime packages?

i'm from rpm world, i don't know about deb world.

@bernd
Copy link
Owner

bernd commented Aug 31, 2015

I guess the libpng* example was a bad one. 😉 The main idea was that you don't have to duplicate dependencies in depends and build_depends.

@glensc
Copy link
Contributor

glensc commented Aug 31, 2015

you can use variables and use it for both lists, if you really need to define explictly runtime libraries, i still consider it packaging bug if -dev package does not pull in runtime package containing the shared lib.

all_depends = %w[libpng0 libjpeg6]

depends all_depends
build_depends all_depends, 'libpng-dev', 'libjpeg-dev'

btw, in rpm world shared library depenencies are generated automatically based on library SONAME:

➔ rpm -q libpng --provides
libpng16.so.16()(64bit)
libpng16.so.16(PNG16_0)(64bit)

$ rpm -q --requires libicns | grep png
libpng16.so.16()(64bit)
libpng16.so.16(PNG16_0)(64bit)

glensc added a commit to pld-linux/fpm-cookery that referenced this pull request Aug 31, 2015
in pld all -dev packages require appropriate runtime packages

bernd/fpm-cookery#123
bernd/fpm-cookery#116 (comment)
@bernd
Copy link
Owner

bernd commented Aug 31, 2015

As said, the libpng example was not optimal and purely made up. In debian the libpng12-dev package pulls in the library as dependency.

@boc-tothefuture boc-tothefuture mentioned this pull request Sep 1, 2015
@boc-tothefuture
Copy link
Author

#126 submitted for option #2.

@glensc
Copy link
Contributor

glensc commented Sep 2, 2015

imho if #126 is merged, default fpm-cookery run should be changed to install build deps only (that option 2). as how to run fpm-cookery without installing "runtime" deps?

@boc-tothefuture
Copy link
Author

@glensc Can you clarify your comment a bit? Why is having the user specify --no-deps during the package phase insufficient? I don't have a long enough history w/ the project to understand all the user cases.

@glensc
Copy link
Contributor

glensc commented Sep 2, 2015

@Brian-OConnell if i want to use fpm-cookery without installing runtime depends (the depends directive) at build time, how do i proceed?

@boc-tothefuture
Copy link
Author

@glensc I believe you specify --no-deps on the command line before your build. It wasn't working for omnibus packages, but I opened issue #124 and related #125 to fix it.

@glensc
Copy link
Contributor

glensc commented Sep 3, 2015

@Brian-OConnell but --no-deps would skip installing build_depends and depends both? i would like build_depends still to be satisfied.

@boc-tothefuture
Copy link
Author

@glensc Before you ran fpm-cook package --no-deps you would run fpm-cook install-build-deps. That command is added in #126
Does that solve you problem?

@glensc
Copy link
Contributor

glensc commented Sep 3, 2015

@Brian-OConnell but i need package as result, thus, i need to call fpm-cook package again and that will call install-deps, so:

  1. fpm-cook install-build-deps.
  2. fpm-cook package --no-deps

@boc-tothefuture
Copy link
Author

@glensc Correct, it would be a two step process at which point you would get a package at the end.

@glensc
Copy link
Contributor

glensc commented Sep 3, 2015

i would prefer if it's one call, maybe add --no-runtime-deps option?

altho i would really preferred option 1 from #116 (comment)

@boc-tothefuture
Copy link
Author

@glensc hmmm. I think we might need to get @bernd opinion on this as well.

@bernd
Copy link
Owner

bernd commented Sep 10, 2015

I think it would be good to just disable the automatic dependency installation for fpm-cook package since we now have install-deps and install-build-deps available. Installing dependencies requires superuser privileges so I think it's good to separate the tasks.

So my proposal is:

  1. Disable the automatic installation of build/runtime depends on fpm-cook package
  2. Add --install-build-deps option to install only the build dependencies on fpm-cook package.
  3. Add --install-deps option to install build and runtime dependencies on fpm-cook package.
  4. Remove the --no-deps options because it's not needed anymore.

Any feedback on that?

@boc-tothefuture
Copy link
Author

@bernd Your proposal is consistent with how I use FPM cookery.

@beddari
Copy link
Contributor

beddari commented Sep 10, 2015

This makes sense, go for it.

@glensc
Copy link
Contributor

glensc commented Sep 11, 2015

👍

bernd added a commit that referenced this pull request Sep 14, 2015
- Remove --no-deps option.
- Add --install-build-depends option to install build dependencies only.
- Add --install-depends option to install all declared dependencies.
- Disable automatic dependency installation on "fpm-cook package". Use
  the new --install-depends/--install-build-depends options to install
  dependencies and build the project with one command.

Based on discussions in #116
@bernd
Copy link
Owner

bernd commented Sep 14, 2015

I created a PR in #131. I am closing this PR now, please review and comment in there. Thank you!

@bernd bernd closed this Sep 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants