| Summary: | Use libacl instead of spawning setfacl(1) | ||
|---|---|---|---|
| Product: | udisks | Reporter: | Samuli Suominen <ssuominen> |
| Component: | general | Assignee: | David Zeuthen (not reading bugmail) <zeuthen> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | medium | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux (All) | ||
| See Also: | http://bugs.gentoo.org/show_bug.cgi?id=412377 | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
setfacl(1) is used at runtime, there is no point in detecting if the build system has it - instead you want to add a runtime dep to your package spec file or similar. So technically this bug can be closed as INVALID.... however, since we really should just be using libattr instead of spawning a command, I'm repurposing this bug that - hope that's OK with you. Actually, it's libacl, not libattr (In reply to comment #1) > setfacl(1) is used at runtime, there is no point in detecting if the build > system has it - instead you want to add a runtime dep to your package spec file > or similar. So technically this bug can be closed as INVALID.... however, since > we really should just be using libattr instead of spawning a command, I'm > repurposing this bug that - hope that's OK with you. When adding udisks2, as a packager, I did various methods of tracking down the dependencies (including reading all autotools build system files, followed by scanelf for NEEDED entries, followed by various source tree greps, etc. None of these turned setfacl command. It was only after users reported it to be failing at runtime. So at very least, this should be documented in, eg. INSTALL doc file or somesuch. What you described using the lib, instead of command, sounds fine to me What about making it optional? Some fallback for systems with no acl/attr related tools/libs? This is not important to *me* but there are many people trying to keep system minimal as possible. Overall, I'm fine with everything -- just long as it's more clear. Fixed in master http://cgit.freedesktop.org/udisks/commit/?id=15250f35ff8770389cc579c304fbcac9beebc203 Btw, there's a bunch of other tools that we spawn at runtime including parted, sfdisk, sgdisk, mount, umount, cryptsetup, swapon, swapoff. It would be helpful to have this in README under a new section called RUNTIME DEPENDENCIES or something. Patches welcome - thanks! (FWIW, I generally don't like making things dependencies optional - and in the setfacl/libacl case it really can't be made optional.) (In reply to comment #5) > Btw, there's a bunch of other tools that we spawn at runtime including parted, > sfdisk, sgdisk, mount, umount, cryptsetup, swapon, swapoff. It would be helpful > to have this in README under a new section called RUNTIME DEPENDENCIES or > something. Patches welcome - thanks! > > (FWIW, I generally don't like making things dependencies optional - and in the > setfacl/libacl case it really can't be made optional.) Thanks David, I'll see if I can come up with a nice list. Give me a day. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
src/udiskslinuxfilesystem.c in git and udisks-1.94.0 has this code: /* Then set the ACL such that only $USER can actually access it */ escaped_user_name = udisks_daemon_util_escape (user_name);; s = g_strdup_printf ("setfacl -m \"u:%s:rx\" \"%s\"", escaped_user_name, mount_dir); but configure.ac is missing a check for the command. it should propably fail if the command is not found because as per this bug report: http://bugs.gentoo.org/412377 it's actually preventing from volumes getting mounted. the user in the downstream bug is also asking for this depend to be made optional. i'm just going to quote him here: "After upgrading to gnome-base/gvfs-1.12.0 volumes fail to mount e. g. in nautilus with this error message: Unable to mount <volume> Failed to execute child process "setfacl" (No such file or directory) The obvious cause of this is that I do not have sys-apps/acl installed as ACLs are not compiled into the filesystems in my kernel. gvfs should not use ACL utilities if not present or at least support USE="-acl". Downgrading to gnome-base/gvfs-1.10.1 solves this (for now). Reproducible: Always Steps to Reproduce: 1. emerge =gnome-base/gvfs-1.12.0 2. Open nautilus 3. Click a mountable volume that is not mounted yet Actual Results: Volume is not mounted, fails with error message as stated above. Expected Results: Should not depend on ACLs being available."