Bug 240138

Summary: [NEW PORT] sysutils/libudisks: Library from udisks project, to access and manipulate disks, storage devices and technologies
Product: Ports & Packages Reporter: PauAmma <pauamma>
Component: Individual Port(s)Assignee: freebsd-ports-bugs (Nobody) <ports-bugs>
Status: Closed FIXED    
Severity: Affects Only Me CC: arrowd, lantw44, pauamma
Priority: --- Keywords: feature
Version: LatestFlags: pauamma: maintainer-feedback+
Hardware: Any   
OS: Any   
URL: https://github.com/storaged-project/udisks
See Also: https://github.com/storaged-project/udisks/pull/693
Bug Depends on:    
Bug Blocks: 240110    
Description Flags
Patch for new port sysutils/libudisks
Should take care of all changes requested.
Second round of changes none

Description PauAmma freebsd_triage 2019-08-27 01:17:02 UTC
Since it appears that bug 240110 will require using udisks2 instead of devel/libgudev and devel/libudev-devd as I initially thought, I or someone should port it. I'm not sure udisks2 belongs in the devel/ category, but I put it there on the grounds that it's a sorta-kinda replacement for these 2 libraries.

Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2019-08-27 02:50:24 UTC
Most ports matching disk* are in the sysutils category, so that's probably a better primary category for it. 

We try to avoid devel unless its specifically and only development related, and only then if there's not a better primary category (where devel can be secondary)
Comment 2 Gleb Popov freebsd_committer 2019-08-27 07:29:30 UTC
UDisks contains Linux-specific backed part. When considering porting it, I decided that it is easier to implement the same service for FreeBSD from scratch. This resulted in sysutils/bsdisks port. It implements minimal functionality required to mount volumes at the moment.

The libudisks part of UDisks project is a D-Bus bindings library, and should be portable.
Comment 3 Ting-Wei Lan 2019-08-27 08:48:24 UTC
(In reply to Gleb Popov from comment #2)
Has anyone asked the upstream if they are willing to accept patches to build just the client library or add FreeBSD-specific code? If the upstream accepts patches for non-Linux operating systems, I think it will be nice to have at least the client library, so we can stop converting projects using the client library to do D-Bus calls directly.

I just ran 'readelf -d /usr/libexec/udisks2/udisksd' on Linux, and I found it needs GLib. It may be a problem on FreeBSD because FreeBSD geom library is known to conflict with GLib. Both FreeBSD geom library and GLib use 'g_' prefix for their function names, which is known to cause problems for libgtop and force us to disable all code using geom in libgtop.
Comment 4 PauAmma freebsd_triage 2019-08-27 22:34:14 UTC
(In reply to Ting-Wei Lan from comment #3)
OK, this sounds to me like you and Gleb Popov are suggesting to do the following, not necessarily in that order:

1- split off libudisks from udisks2,
2- patch it so it can use bsdisks instead,
3- offer those patches to upstream for their consideration.

If my understanding is correct, who's in charge of drafting an email to the udisks2 upstream? Me? (I don't want to tread on anyone's toes, but I don't want this bug to sit around gathering dust if no one else has time to move on it.)
Comment 5 PauAmma freebsd_triage 2019-08-27 22:36:48 UTC
(In reply to Kubilay Kocak from comment #1)
Is the "maintainer feedback" flag asking me to agree with the change from devel/ to sysutils/ ? If so, how do I say I agree (which I do)? Change it to + ?
Comment 6 Gleb Popov freebsd_committer 2019-08-28 07:07:05 UTC
(In reply to PauAmma from comment #4)
You shouldn't need p.2 at all and regarding ps. 1 and 3, I'd recommend you do the following:

- name the port sysutils/libudisks
- use UDisks2 distfile
- patch configure script to pass on FreeBSD (it looks for udev, which is not applicable for us)
- build only libudisks target and install its products

In ideal case, this shouldn't even require communicating with upstream.

I'd do this myself, but I'm away from computer ATM.
Comment 7 Ting-Wei Lan 2019-08-29 08:14:55 UTC
(In reply to Gleb Popov from comment #6)
It is not required, but it is usually better to let the upstream know that there are people interested in building only the client library. Ideally, all required patches will be merged upstream and no patch will be required in the port.
Comment 8 Ting-Wei Lan 2019-09-08 04:23:37 UTC
I just submitted a pull request to upstream to allow building without the Linux-only udisks daemon: https://github.com/storaged-project/udisks/pull/693. It can be built by passing --disable-daemon --disable-gtk-doc to configure, but there are a few warnings when using the client library with bsdisks.

(process:23042): GLib-GIO-WARNING **: 12:21:10.961: Trying to set property Configuration of type aa{sv} but according to the expected interface the type is a(sa{sv})

(process:23042): GLib-GIO-WARNING **: 12:21:10.961: Trying to set property Symlinks of type as but according to the expected interface the type is aay
Comment 9 Gleb Popov freebsd_committer 2019-09-08 10:03:03 UTC
(In reply to Ting-Wei Lan from comment #8)

Great news!

> there are a few warnings when using the client library with bsdisks.

Should be fixed in bsdisks HEAD revision. Please try it out.
Comment 10 Ting-Wei Lan 2019-09-08 10:23:06 UTC
(In reply to Gleb Popov from comment #9)
The warning for Symlinks is now resolved, but Configuration still shows warnings.

(gnome-control-center:64166): GLib-GIO-WARNING **: 18:22:04.156: Trying to set property Configuration of type (sa{sv}) but according to the expected interface the type is a{sv}
Comment 11 Gleb Popov freebsd_committer 2019-09-08 10:39:27 UTC
(In reply to Ting-Wei Lan from comment #10)
Should be fixed too now. Thanks for testing.
Comment 12 Ting-Wei Lan 2019-09-08 11:35:35 UTC
(In reply to Gleb Popov from comment #11)
Yes, it is fixed. There is no warning now.
Comment 13 PauAmma freebsd_triage 2019-09-08 13:22:37 UTC
(In reply to Ting-Wei Lan from comment #8)
Wow! You're way faster that me. Thanks. *goes look at PR to see what needed to be done*
Comment 14 Ting-Wei Lan 2019-10-03 15:11:12 UTC
https://github.com/storaged-project/udisks/pull/693 has been accepted and merged by upstream.
Comment 15 PauAmma freebsd_triage 2019-10-04 20:13:17 UTC
Created attachment 208102 [details]
Patch for new port sysutils/libudisks

Tested against sysutils/bsdisks
Also tested with:
- portlint -AN
- port test from porttools (the upstream head, https://github.com/skreuzer/porttools)
- poudriere
Comment 16 Gleb Popov freebsd_committer 2019-10-06 08:29:04 UTC
Patches in files/ are huge. Why do you need all these? I had an impression that there should be only some tweaks in autotools stuff.
Comment 17 Ting-Wei Lan 2019-10-06 09:08:48 UTC
(In reply to Gleb Popov from comment #16)
Upstream requires gtk-doc build to work even if --disable-daemon is used. To fix gtk-doc build, functions and types which are parts of the daemon have to be moved to different files. Otherwise, gtk-doc will complain that it can't find some functions and types when the daemon isn't built.

There are totally 8 commits in the pull request.
Comment 18 Ting-Wei Lan 2019-10-06 09:23:23 UTC
Comment on attachment 208102 [details]
Patch for new port sysutils/libudisks

>+# pkaction takes care of libpolkit-agent-1 and libpolkit-gobject-1
>+# which are what we really want (see UDISKSCTL_LIB_DEPENDS).
>+BUILD_DEPENDS=	bash:shells/bash gtkdocize:textproc/gtk-doc \

Is there any reason to put two dependencies in one line here?

>+UDISKSCTL_DESC=	Install udisksctl command line utility
>+UDISKSCTL_LIB_DEPENDS=	libpolkit-agent-1.so:sysutils/polkit \
>+			libpolkit-gobject-1.so:sysutils/polkit

Can we enable UDISKSCTL by default? It is just a small tool that is useful for testing. It also makes the port similar to the upstream default because the upstream project doesn't have an option to disable udisksctl.

>+# config.status errors out without --disable-dependency-tracking, with or
>+# without USES=gmake.
>+	cd ${WRKSRC} && \
>+	./autogen.sh --disable-daemon --disable-gtk-doc --disable-nls \

Why --disable-nls is used? If you think NLS should be optional, put it under NLS option. NLS is enabled by default. --disable-gtk-doc should also be optional, so users can decide if the documentation is needed.
Comment 19 PauAmma freebsd_triage 2019-10-06 22:54:11 UTC
(In reply to Gleb Popov from comment #16)
What Ting-Wei Lan said. Nearly all those patches will go away when udisks 2.9.0 is released and the port updated to it. The only one that may remain is one inspired from devel/libdbusmenu/files/patch-libdbusmenu-glib_Makefile.in that makes the upstream Makefiles respect PREFIX for everything. Otherwise, there are 2 files that would end under /usr/local no matter what.
Comment 20 PauAmma freebsd_triage 2019-10-07 03:49:35 UTC
(In reply to Ting-Wei Lan from comment #18)
> Is there any reason to put two dependencies in one line here?

To answer your question as asked: because it fit within the 80 columns of my terminal window and because neither the Porter's Handbook nor portlint said to do it otherwise. (Should they?) But to answer it as you probably meant it: since I have to make the other changes you requested, I'll split that line as well.
Comment 21 PauAmma freebsd_triage 2019-10-10 04:45:58 UTC
Created attachment 208219 [details]
Should take care of all changes requested.
Comment 22 Gleb Popov freebsd_committer 2019-10-10 07:21:00 UTC

GH_TAGNAME=	udisks-2.8.4

really required? Looks like default value.

Other than that looks good to me. I'm a bit hesitant to commit such big patches in files/ but waiting for next release is an even worse idea, I think.
Comment 23 PauAmma freebsd_triage 2019-10-10 11:49:00 UTC
(In reply to Gleb Popov from comment #22)

Without it (and with distfiles adjusted to reflect the changed filename) I get:

% make fetch
===>  License LGPL20+ accepted by the user
===>   libudisks-2.8.4 depends on file: /usr/local/sbin/pkg - found
=> storaged-project-udisks-2.8.4_GH0.tar.gz doesn't seem to exist in /usr/home/pauamma/ports/distfiles/.
=> Attempting to fetch https://codeload.github.com/storaged-project/udisks/tar.gz/2.8.4?dummy=/storaged-project-udisks-2.8.4_GH0.tar.gz
fetch: https://codeload.github.com/storaged-project/udisks/tar.gz/2.8.4?dummy=/storaged-project-udisks-2.8.4_GH0.tar.gz: Not Found
=> Attempting to fetch http://distcache.FreeBSD.org/ports-distfiles/storaged-project-udisks-2.8.4_GH0.tar.gz
fetch: http://distcache.FreeBSD.org/ports-distfiles/storaged-project-udisks-2.8.4_GH0.tar.gz: Not Found
=> Couldn't fetch it - please try to retrieve this
=> port manually into /usr/home/pauamma/ports/distfiles/ and try again.
*** Error code 1

With it, make fetch works normally:

% make fetch
===>  License LGPL20+ accepted by the user
===>   libudisks-2.8.4 depends on file: /usr/local/sbin/pkg - found
=> storaged-project-udisks-2.8.4-udisks-2.8.4_GH0.tar.gz doesn't seem to exist in /usr/home/pauamma/ports/distfiles/.
=> Attempting to fetch https://codeload.github.com/storaged-project/udisks/tar.gz/udisks-2.8.4?dummy=/storaged-project-udisks-2.8.4-udisks-2.8.4_GH0.tar.gz
fetch: https://codeload.github.com/storaged-project/udisks/tar.gz/udisks-2.8.4?dummy=/storaged-project-udisks-2.8.4-udisks-2.8.4_GH0.tar.gz: size unknown
fetch: https://codeload.github.com/storaged-project/udisks/tar.gz/udisks-2.8.4?dummy=/storaged-project-udisks-2.8.4-udisks-2.8.4_GH0.tar.gz: size of remote file is not known
storaged-project-udisks-2.8.4-udisks-2.8.4_GH0        1345 kB 1229 kBps    02s
===> Fetching all distfiles required by libudisks-2.8.4 for building
Comment 24 Ting-Wei Lan 2019-10-11 03:56:09 UTC
Comment on attachment 208219 [details]
Should take care of all changes requested.

>Index: sysutils/libudisks/pkg-plist
>--- sysutils/libudisks/pkg-plist	(nonexistent)
>+++ sysutils/libudisks/pkg-plist	(working copy)
>@@ -0,0 +1,149 @@

Why gir file is only installed when DOCS is enabled? I know gir file isn't needed during runtime, but I think the convention is to always install it when gobject-introspection is enabled.
Comment 25 Ting-Wei Lan 2019-10-11 04:07:18 UTC
(In reply to Gleb Popov from comment #22)
It is required because the tag is udisks-2.8.4, not just 2.8.4.

But probably we can use


instead of setting GH_TAGNAME so we don't have to write the version number twice in the same file.
Comment 26 PauAmma freebsd_triage 2019-10-12 01:45:09 UTC
Created attachment 208253 [details]
Second round of changes
Comment 27 Ting-Wei Lan 2019-10-12 03:54:12 UTC
(In reply to PauAmma from comment #26)
The patch looks good to me now. The only remaining problem I found is likely a bsdisks bug: 'udisksctl dump' shows nothing when it is run the first time. Running 'udisksctl dump' again shows the correct result.
Comment 28 commit-hook freebsd_committer 2019-10-12 18:56:40 UTC
A commit references this bug:

Author: arrowd
Date: Sat Oct 12 18:55:52 UTC 2019
New revision: 514350
URL: https://svnweb.freebsd.org/changeset/ports/514350

  sysutils/libudisks: New port. A client library to UDisks daemon, which some applications are using to interface with UDisks.

  PR:		240138

Comment 29 Gleb Popov freebsd_committer 2019-10-12 18:58:24 UTC
Thanks everyone involved.