Wrong use of $(DESTDIR) causing build failures in package managers

[hasse69]

Makefile.am is trying to remove /sbin/mount.rar2fs without $(DESTDIR) and thus, package managers will of course forbid this and throw an error
Only $(DESTDIR) can be worked on, and then the files get merged to the actual file system
Also, the symlink creation in Makefile.am is wrong way around
The attached patch fixes things for me (and Gentoo and any distribution using sandbox in their packaging, so pretty much any)

Original issue reported on code.google.com by ssuominen.private on 2013-03-13

[hasse69]

Earlier release 1.15.1 was okay, so this problem is new with 1.16.0

Original issue reported on code.google.com by ssuominen.private on 2013-03-13 07:52:07

[hasse69]

Looks like this after fixing the order of LN_S:

-rwxr-xr-x 1 root root 10344 Mar 13 09:48 /usr/bin/mkr2i
lrwxrwxrwx 1 root root     6 Mar 13 09:48 /usr/bin/mount.rar2fs -> rar2fs
-rwxr-xr-x 1 root root 74616 Mar 13 09:48 /usr/bin/rar2fs

Just to proof the case ;-)

And if you are intrested, this is the sandbox error, it might vary a bit based on distro,
but almost same anyways:

d /sbin && \
rm -f mount.rar2fs && \
ln -s /var/tmp/portage/sys-fs/rar2fs-1.16.0/image//usr/bin/rar2fs mount.rar2fs
 * ACCESS DENIED:  unlinkat:     /sbin/mount.rar2fs
rm: cannot remove ‘mount.rar2fs’: Permission denied
make[2]: *** [install-exec-hook] Error 1
make[2]: Leaving directory `/var/tmp/portage/sys-fs/rar2fs-1.16.0/work/rar2fs-1.16.0'
make[1]: *** [install-exec-am] Error 2
make[1]: Leaving directory `/var/tmp/portage/sys-fs/rar2fs-1.16.0/work/rar2fs-1.16.0'
make: *** [install-am] Error 2

Original issue reported on code.google.com by ssuominen.private on 2013-03-13 07:58:47

[hasse69]

Thanks for report. This change sneaked through QA testing since I completely forgot
about it and never used it myself. I will apply the patch and release 1.16.1 asap.

Hans

Original issue reported on code.google.com by hasse69 on 2013-03-13 11:32:10

[hasse69]

Can you double check that this also works

$(LN_S) rar2fs $(DESTDIR)$(sbindir)/mount.rar2fs

mount support scripts should by tradition be located in sbin not bin.

Original issue reported on code.google.com by hasse69 on 2013-03-13 12:05:07

[hasse69]

Another question is why you had to remove
   rm -f mount.rar2fs
IIRC $(LN_S) is "ln -s", and not "ln -sf" which means installation will fail if the
file already exists. I think it would be safer to also add
   rm -f $(DESTDIR)$(sbindir)/mount.rar2fs
before calling $(LN_S)

Original issue reported on code.google.com by hasse69 on 2013-03-13 18:42:42

[hasse69]

Ok, I think this is the correct logic in Makefile.am

if LINUX
install-exec-hook:
        rm -f $(DESTDIR)$(sbindir)/mount.rar2fs && \
        $(LN_S) $(DESTDIR)$(bindir)/rar2fs $(DESTDIR)$(sbindir)/mount.rar2fs
endif

The proposed patch would only work if rar2fs and the mount script was located in the
same directory. The 'rm' is needed. At least when I tried it on a local install, not
using a deploy mechanism. If the package manager always starts from scratch it is not
an issue. But for plain 'make install' it would generate an ugly error message the
second time if rar2fs was not uninstalled in between.

I will commit this change to the trunk. Can you please verify if the latest from trunk
works as expected in your environment? Then I can release 1.16.3.

Hans

Original issue reported on code.google.com by hasse69 on 2013-03-13 19:52:00

[hasse69]

No feedback from submitter. It is assumed that latest version in trunk will fix this
issue. A patch file is provided that can be applied on the 1.16.0 version of this file
until a new official release is made available.

Original issue reported on code.google.com by hasse69 on 2013-03-14 19:20:28

[hasse69]

A patch file is now available in the download section.
The fix has also been applied to the trunk.

Original issue reported on code.google.com by hasse69 on 2013-03-15 20:59:29

[hasse69]

This won't work.

The way DESTDIR works is that package managers set temporary directory like
DESTDIR=/tmp/foobar, so in this case what would happen is:

$ ln -s /tmp/foobar/usr/bin/rar2fs /tmp/foobar/usr/sbin/mount.rar2fs

And then content of /tmp/foobar gets copied into the live file system, and
/usr/sbin/mount.rar2fs points to non-existing file
/tmp/foobar/usr/bin/rar2fs

So it's OK to leave rm -f $(DESTDIR)$(sbindir)/mount.rar2fs if you want,
althought DESTDIR is always empty, so it doesn't make a difference when
installing from package manager
But having double DESTDIR that makes the path hardcoded (non-relative) is
not

So this would be correct:

if LINUX
install-exec-hook:
rm -f $(DESTDIR)$(sbindir)/mount.rar2fs && \
$(LN_S) $(bindir)/rar2fs $(DESTDIR)$(sbindir)/mount.rar2fs
endif

Original issue reported on code.google.com by ssuominen.private on 2013-03-17 08:45:42

[hasse69]

And to clarify about the location of the mount/umount wrappers. Current mount/umount
from util-linux can find the mount.* umount.* wrappers from both, /bin, /sbin, /usr/bin,
/usr/sbin, ... As in, it doesn't matter thesedays because some distributions have started
merging the sbin into bin, and therefore util-linux upstream added all of the directories
to the search paths

You could even claim "bin is the new sbin", although some old unix beards would shoot
me for saying so... heh
Personally I don't care at this time yet ;-)

Original issue reported on code.google.com by ssuominen.private on 2013-03-17 08:52:49

[hasse69]

Reopening the case.

There is something not right here. We can not implement the install-hook to suit a
specific package manager. It must be generic and work also for a standard install.
I am not too sure your last suggestion will do that. Also, something does not look
right. If DESTDIR is set to /tmp then that must never be used in the link! It will
break when copied to the real rootfs. 
I need to check how automake install hook is supposed to handle this. I believe that
automake is using whatever paths have been specified at configure time. That means
that if no prefix is used, standard paths are used. Otherwise --prefix should be used.
It is the job of the package manager to configure the package properly. So this should
be completely opaque to the install hook.

Original issue reported on code.google.com by hasse69 on 2013-03-17 09:29:50

[hasse69]

Yes, now it makes sense. --prefix and destdir are two completely different beasts. The
package manager will provide the destdir. Normal installs will not . So your last suggested
patch will work in both scenarios. 
Thanks. I will update the patch and submit a new version to trunk.
Also, The rm is mainly there for normal installs but for some build systems it might
also be useful when wanting to clean up a staging area.

Original issue reported on code.google.com by hasse69 on 2013-03-17 10:29:04

[hasse69]

A new patch file has been uploaded to the download section.
The fix has also been committed to trunk.

Original issue reported on code.google.com by hasse69 on 2013-03-17 11:50:22

[hasse69]

trunk is broken again, did svn checkout today and:

rm -f /var/tmp/portage/sys-fs/rar2fs-1.16.0_p20130509/image//usr/sbin/mount.rar2fs
&& \
ln -s /usr/bin/rar2fs /var/tmp/portage/sys-fs/rar2fs-1.16.0_p20130509/image//usr/sbin/mount.rar2fs
ln: failed to create symbolic link ‘/var/tmp/portage/sys-fs/rar2fs-1.16.0_p20130509/image//usr/sbin/mount.rar2fs’:
No such file or directory

Original issue reported on code.google.com by ssuominen.private on 2013-05-09 17:59:58

[hasse69]

ah, this time it's missing:

$(MKDIR_P) $(DESTDIR)$(sbindir) before trying to use it in Makefile.am
add AC_PROG_MKDIR_P to configure.ac

Original issue reported on code.google.com by ssuominen.private on 2013-05-09 18:08:27

[hasse69]

Patch for the missing $(sbindir) in $(DESTDIR)

Original issue reported on code.google.com by ssuominen.private on 2013-05-09 18:13:08

---

- _Attachment: [rar2fs-1.16.0_p20130509-destdir.patch](https://storage.googleapis.com/google-code-attachments/rar2fs/issue-12/comment-17/rar2fs-1.16.0_p20130509-destdir.patch)_

[hasse69]

Thanks for the patch. Applied to trunk.

Original issue reported on code.google.com by hasse69 on 2013-05-09 20:56:02

[hasse69]

I can confirm that trunk r369 fixes the issues with DESTDIR. Any chance of getting a
release of this still?

Original issue reported on code.google.com by vadmium on 2013-05-26 08:53:41

[hasse69]

Yes, it would be possible to release a new version with RAR5 support.
Just have not had the time to do it yet. Will look at it ASAP.

Original issue reported on code.google.com by hasse69 on 2013-05-26 09:27:53

[hasse69]

I can confirm that trunk r369 fixes the issues with DESTDIR. Any chance of getting a
release of this still?

Original issue reported on code.google.com by vadmium on 2013-05-26 08:53:41

[hasse69]

Yes, it would be possible to release a new version with RAR5 support.
Just have not had the time to do it yet. Will look at it ASAP.

Original issue reported on code.google.com by hans.beckerus on 2013-05-26 09:27:53

[hasse69]

(No text was entered with this change)

Original issue reported on code.google.com by hans.beckerus on 2013-07-28 19:28:33