search

jmmv / pkg_comp

Automates the build of pkgsrc binary packages in a sandbox
BSD 3-Clause "New" or "Revised" License
15 stars 3 forks source link

mk.conf has a sandbox-internal path in it #4

Closed truist closed 6 years ago

truist commented 7 years ago

Per my comment on your blog post:

Then, because of that symlink (which I had to recreate, but I didn't realize that until after I'd gone a ways down the path without it), I ended up with an /etc/mk.comp (from bootstrap) that has an inside-the-sandbox path in it. That feels a little weird, but I haven't figured out a better option so I'm just living with it.

To rephrase: the mk.conf that is extracted from the bootstrap archive has .sinclude "/pkg_comp/pkg.mk.conf" in it. That path only makes sense from inside the sandbox, but that file ends up in a place where it has an effect outside the sandbox. It's confusing, and worst-case someone could end up with a matching path and unintended consequences.

jmmv commented 7 years ago

I've been taking a look at this... and the solution may be uglier than what's already happening.

The problem is that pbulk likes to clean the "build client" at startup, and then re-extracts the bootstrap kit. This means that the mk.conf file is always fresh at the beginning of each build, and that the contents of mk.conf are determined by what's inside bootstrap.tgz — which in turn means that mk.conf must contain all correct settings for package building from the very beginning and that configuring the file post-bootstrap in a sandbox-specific way doesn't work.

The only solution I see is to:

  1. Run bootstrap without --mk-fragment so that the bootstrap process generates a "clean" mk.conf file.
  2. Make pkg_comp extract the bootstrap.tgz file in a temporary location.
  3. Modify the extracted mk.conf file to include the sandbox-specific configuration.
  4. Repack the extracted/modified contents into a bootstrap-sandbox.tgz file.
  5. Point pbulk at using bootstrap-sandbox.tgz instead of bootstrap.tgz.

The reason this is ugly is because bootstrap-sandbox.tgz will either be leaked to the packages tree, or this file will have to be recreated on each pkg_comp invocation.

Alternatively... I could modify pbulk to support an extra "mk fragment" and let it deal with it. But that doesn't seem easy either.

jmmv commented 7 years ago

I've just uploaded a possible fix for this. Will merge after testing passes, and possibly tomorrow anyway. Please take a look.

truist commented 7 years ago

I'll look at the diff one evening soon. One thought, first - I have already put .sinclude "/etc/pkg_comp/extra.mk.conf" into /etc/mk.conf, so that I can get e.g. make show-options to work correctly outside of the sandbox. That isn't strictly related to this issue (i.e. the /pkg_comp/ include is still odd and has a slim chance of grabbing the wrong file), but it makes it clear to me that there is a maintenance issue here - given a default install of pkg_comp, where should I put my package-specific settings? And if I put them into e.g. /etc.pkg_comp/extra.mk.conf, how do I get /etc/mk.conf to know about them?

What if, instead of replacing /etc/mk.conf with a pkg_comp-specific one (in the sandbox), you appended all the settings you need to override (to the one in the sandbox), and kept a hard reference to e.g. /etc/pkg_comp/extra.mk.conf in /etc/mk.conf? Or even kill off /etc/pkg_comp/extra.mk.conf entirely, just use /etc/mk.conf, and override the relevant settings (by appending to the file) in the sandbox?

jmmv commented 6 years ago

Alright! After a really long time, I retook the patch I was playing with, made it work (after rewriting it from scratch...) and submitted a fix. I think this should work.

(The reason I got stuck at first is because I had to shard the integration tests so that they didn't run over Travis CI limits, and I was pretty lazy to do so. Turns out this wasn't that hard.)