Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 466554 - multibuild.eclass: flock not available on OSX (GNU userspace)
Summary: multibuild.eclass: flock not available on OSX (GNU userspace)
Status: RESOLVED FIXED
Alias: None
Product: Gentoo/Alt
Classification: Unclassified
Component: Prefix Support (show other bugs)
Hardware: All OS X
: Normal normal (vote)
Assignee: Michał Górny
URL: http://article.gmane.org/gmane.linux....
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-20 11:20 UTC by MATSUI Tetsushi
Modified: 2013-05-24 17:42 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description MATSUI Tetsushi 2013-04-20 11:20:04 UTC
Emerging a package using multibuild, for me it is typically a python package,
produces a QA Notice.

Reproducible: Always

Steps to Reproduce:
1. emerge flaggie, for example

Actual Results:  
Here is the end of a log for flaggie:

 * QA Notice: command not found:
 * 
 *      /Users/tetsushi/Gentoo/var/tmp/portage/app-portage/flaggie-0.2-r3/temp/environment: line 2586: flock: command not found
 *      /Users/tetsushi/Gentoo/var/tmp/portage/app-portage/flaggie-0.2-r3/temp/environment: line 2586: flock: command not found
ecompressdir: bzip2 -9 /usr/share/doc

Expected Results:  
Successful emerge without QA Notice.

There is no flock(1) on OS X; the system does not provide it and sys-apps/util-linux does not install it.
The eclass, however,  expects its presence on userland_GNU.
Comment 1 Fabian Groffen gentoo-dev 2013-05-07 19:43:37 UTC
multibuild_merge_root IMO shouldn't rely on userland_* stuff, instead it should test on availability of commands, e.g. lockf, flock, lockfile.

Last but not least, it should probably use a generic method for locking, using the shell only (not requiring any external tools).

e.g.

touch waiting.$$ && until ln waiting.$$ waiting 2>/dev/null; do sleep 1; done && rm waiting.$$
do stuff
rm waiting
Comment 2 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-05-07 21:08:26 UTC
@ulm, you were the one suggesting using userland, weren't you? Any suggestions?

@Fabian, thanks for the tip. I will think it thoroughly in the next few days.
Comment 3 Ulrich Müller gentoo-dev 2013-05-07 21:56:12 UTC
(In reply to comment #2)
> @ulm, you were the one suggesting using userland, weren't you? Any
> suggestions?

Your original logic in multibuild_merge_root() was testing for availability of lockf to determine if it's BSD or GNU:

+     if type -P lockf &>/dev/null; then
+             # On BSD, we have 'lockf' wrapper.
+             tar -C "${src}" -f - -c . \
+                     | lockf "${lockfile}" tar -x -f - -C "${dest}"
+     else
+ [...]
+             cp -a -l -n "${src}"/. "${dest}"/

My suggestion was merely that you should test for GNU userland when you require the GNU version of commands (like cp -a).
Comment 4 Fabian Groffen gentoo-dev 2013-05-08 05:47:23 UTC
(In reply to comment #3)
> My suggestion was merely that you should test for GNU userland when you
> require the GNU version of commands (like cp -a).

+1.  For that, userland seems fine, since you have not much else to test that on (and should only happen for Gentoo/*BSD guys).

If you stick that conditional within the shell-based portable lock code, you'll be fine on all (UNIX-like) Prefix platforms.
Comment 5 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-05-08 17:26:09 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > My suggestion was merely that you should test for GNU userland when you
> > require the GNU version of commands (like cp -a).
> 
> +1.  For that, userland seems fine, since you have not much else to test
> that on (and should only happen for Gentoo/*BSD guys).

Well, the main reason for userland_ conditional was that BSD 'cp' was (is?) broken and didn't work correctly with '--no-clobber' (though it supports that option).

> If you stick that conditional within the shell-based portable lock code,
> you'll be fine on all (UNIX-like) Prefix platforms.

At a third glance, the code looks fine. Are there any potential issues that I should be aware of?
Comment 6 Fabian Groffen gentoo-dev 2013-05-08 18:28:43 UTC
(In reply to comment #5)
> Well, the main reason for userland_ conditional was that BSD 'cp' was (is?)
> broken and didn't work correctly with '--no-clobber' (though it supports
> that option).

I now understand that

> > If you stick that conditional within the shell-based portable lock code,
> > you'll be fine on all (UNIX-like) Prefix platforms.
> 
> At a third glance, the code looks fine. Are there any potential issues that
> I should be aware of?

Nope, apart from that you should make sure the try file and the actual lock file are on the same filesystem, but that should be easy to assure.
Comment 7 Toralf Förster gentoo-dev 2013-05-16 16:07:50 UTC
FWIW, locking a file is racy (at least in theory) - while creating a directory is guaranteed to be an atomar operation by the kernel, therefore creating a directory to hold a lock is the better choice.

http://www.linux-magazin.de/Ausgaben/2012/05/Bash-Bashing/%28language%29/ger-DE


/just my 2ct
Comment 8 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-05-23 20:19:32 UTC
Committed.
Comment 9 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-05-24 17:32:03 UTC
@Fabian, I think the new locking code is causing deadlock sometime (I've seen one today, then bug 471168 may be related). Could you double-check it?
Comment 10 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-05-24 17:42:37 UTC
It seems that $$ was responsible. I've replaced it with $BASHPID and hopefully everything will work fine now.