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

refs #16207 - generate custom.yaml symlink during build phase #205

Merged
merged 1 commit into from
Oct 31, 2016

Conversation

domcleal
Copy link
Contributor

Ensures the build phase of the Rakefile can be run with a different
prefix to the installation phase, for use in package builds where the
installation phase prefix is not the standard directory. Matches how
configuration files all have paths set in the build phase.


rake clean build PREFIX=/ SYSCONFDIR=/etc && rake install PREFIX=/tmp/installer on develop currently generates:

-rw-rw-r--. 1 dcleal dcleal 736 Oct 31 15:18 /tmp/installer/etc/foreman-installer/custom-hiera.yaml
lrwxrwxrwx. 1 dcleal dcleal  54 Oct 31 15:18 /tmp/installer/share/foreman-installer/config/foreman.hiera/custom.yaml -> /tmp/installer/etc/foreman-installer/custom-hiera.yaml

but with this patch it generates a symlink pointing to the directory provided at build time:

-rw-rw-r--. 1 dcleal dcleal 736 Oct 31 15:17 /tmp/installer/etc/foreman-installer/custom-hiera.yaml
lrwxrwxrwx. 1 dcleal dcleal  40 Oct 31 15:17 /tmp/installer/share/foreman-installer/config/foreman.hiera/custom.yaml -> /etc/foreman-installer/custom-hiera.yaml

Ensures the build phase of the Rakefile can be run with a different
prefix to the installation phase, for use in package builds where the
installation phase prefix is not the standard directory. Matches how
configuration files all have paths set in the build phase.
@stbenjam
Copy link
Member

The first version looks correct to me, why wouldn't you want the symlink to be correct according to the environment set during install?

@stbenjam
Copy link
Member

Oh I see, it's missing the SYSCONFDIR in the first version. The second one still looks wrong to me...

Why are you giving different paths to the 2 stages?

@domcleal
Copy link
Contributor Author

The second one still looks wrong to me...

Why are you giving different paths to the 2 stages?

We rely on this split in Debian and RPM packaging of foreman-installer to first build the package contents into _build - all of the config files, the symlinks, caches etc - with all paths referenced in configs/symlinks as they'll be in the real installation (e.g. pointing to /etc/foreman-installer), but the files themselves are in _build.

The second phase is where those files are copied into the installation structure, e.g. etc/foreman-installer/scenarios.d/foreman.yaml, usr/share/foreman-installer beneath some packaging build directory. The path has to be different because you can't actually install to / in a package build environment - it's usually read-only. You install to some other dir (%buildroot for RPM) and the contents are copied into the resulting binary package.

If a) you provided the %buildroot to the build phase, you'd get references to the build root in the config files and symlinks, which is what happened here (the symlink points to /builddir), or b) you provided the / to the installation phase (the symlink would now point to /etc) then it would be installed into the build chroot instead of the package build target directory.

(https://github.com/theforeman/foreman-packaging/blob/deb/develop/debian/xenial/foreman-installer/rules, https://github.com/theforeman/foreman-packaging/blob/rpm/develop/foreman-installer/foreman-installer.spec#L50)

Copy link
Member

@stbenjam stbenjam left a comment

Choose a reason for hiding this comment

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

Ah ok, thanks for explaining - clear now. Change looks fine then, and tested locally. It seems to do what you want.

@mmoll mmoll merged commit 9b69685 into theforeman:develop Oct 31, 2016
@mmoll
Copy link
Contributor

mmoll commented Oct 31, 2016

merged, thanks @domcleal!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants