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.

Sign up for GitHub

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

Jump to bottom

make install fails when --prefix needs to be an absolute path #823

Open
aptenodytes-forsteri opened this issue Nov 29, 2021 · 6 comments · May be fixed by #1151
Open

make install fails when --prefix needs to be an absolute path #823

aptenodytes-forsteri opened this issue Nov 29, 2021 · 6 comments · May be fixed by #1151
Assignees
@jsharpe

Comments

@aptenodytes-forsteri
Copy link

I am trying to build nginx from sources. With nginx, it is fairly standard to set --prefix to an absolute path. For example, the default value is /usr/local/nginx.

http://nginx.org/en/docs/configure.html

If I try to use install_prefix to do this, rules_foreign_cc still appends $$BUILD_TMPDIR$$ to my install_prefix. This will make nginx think its config and such are in /some/long/bazel/tmpdir/usr/local/nginx.

It seems that the correct tool to use in this case is DESTDIR. I want to be able to set --prefix like any other arg I pass to configure, then use DESTDIR to install it in the tmp dir.

Here is a patch that seems to accomplish what I want:
https://gist.github.com/aptenodytes-forsteri/99f84a715b481db59442cde579c432b1

Here is an example of building nginx from sources:

filegroup(
    name = "srcs",
    srcs = glob(["**/*"]),
)

configure_make(
    name = "nginx_all",
    args = ["--debug"],
    configure_in_place = True,
    configure_options = [
       # Note that it is standard to have absolute paths here, so --prefix doesn't work.
        "--sbin-path=/usr/sbin/nginx",
        "--modules-path=/usr/lib/nginx/modules",
        "--conf-path=/etc/nginx/nginx.conf",
        "--pid-path=/var/run/nginx/nginx.pid",
        "--lock-path=/var/run/nginx/nginx.lock",
       # Redacted - some other flags to include nginx modules
    ],
    lib_source = ":srcs",
    # This matches the --sbin-path flag above
    out_bin_dir = "usr/sbin",
    out_binaries = ["nginx"],
   # This is the part I added in my patch.
    override_install_prefix = "/usr/share/nginx"
)

cc: @alexeagle
@jsharpe
Copy link
Member

jsharpe commented Nov 30, 2021

Funnily enough I was trying to come up with something very similar - my use case was to set the install_prefix to be under /proc/self/cwd to get reproducible paths baked into the produced binaries but I ran into issues with mkdir failing in the configure script but using DESTDIR for a two phase install may just fix that for me, I'll give that a try and see if I can put in a more generic fix for this as the cmake and make rules will also have similar issues.

@jsharpe jsharpe self-assigned this Nov 30, 2021
@aptenodytes-forsteri
Copy link
Author

Great, thank you!

@jmmv
Copy link

jmmv commented Mar 10, 2023

Arrived to this bug because I was going to report something similar, but I think it fits here.

What I'm trying to do is build some software with Bazel and later zip up the results and generate an installable "package". But this is bound to be very problematic because these rules confuse the prefix with DESTDIR. These are two very different concepts, and they must be handled separately.

The reason is that the installation prefix is often used in the built code to refer to paths in an absolute manner. The installation prefix, thus, must remain unmodified and refer to the "target" location, such as /usr/local or /opt/<package-name>. This way, any computed paths point to the final location. However, when running make install, the call must override DESTDIR to point to a transient location.

Think of it this way: if you are configuring with --prefix=/usr/local, then the installation process should do:

make install DESTDIR=$(pwd)/install

Which will then result in a file structure like the following:

./install/bin/program
./install/share/program/some-file

The artifacts will have the correct paths built into them, but the file layout will be created in a temporary location inside the execroot that can later be distributed in some fashion.

@jsharpe
Copy link
Member

jsharpe commented Mar 10, 2023

@jmmv I started looking at this here: #831. IIRC the problem I ran into is not all configure / make style workflows are autotools based and so DESTDIR doesn't always work. I don't currently have bandwidth to revive those changes but feel free to contribute something along those lines if you do!

@jmmv
Copy link

jmmv commented Mar 10, 2023

@jsharpe Yeah, that's a problem (that not all packages properly respect DESTDIR). In my past life as a pkgsrc maintainer, those were often considered bugs in upstream code that ought to be patched or worked-around with custom "install" scripts. But yeah, whatever is done here should recognize this fact and allow working with such (broken) software.

I think the default in these rules should be to rely on DESTDIR but then offer a way out to do something different for those cases where this doesn't work. This would be a breaking change though, so I'm not sure if you'd like to pursue it as such.

I don't have an /immediate/ need to fix this, but the issue may come to bite us back not too long after, at which point I might attempt a fix and it'd be good to know what the right path forward is :)

@snakethatlovesstaticlibs
Copy link

I also just ran into this issue with the same use case as @jmmv, I'm trying to build a package and redistribute it outside of bazel.

I can see the difficulty of using DESTDIR and I was wondering if it's possible to somehow symlink the prefix directory into bazel's sandbox directory to work around software that doesn't respect DESTDIR?

@mishazharov mishazharov linked a pull request Jan 1, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jsharpe jsharpe

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow prefix to be set as an absolute path in configure mishazharov-forked/rules_foreign_cc
4 participants
@jsharpe @jmmv @aptenodytes-forsteri @snakethatlovesstaticlibs