Re: [Utopia] gnome-vfs HAL patch, take 3

[Alexander Larsson <alexl redhat com>]

On Thu, 2004-07-08 at 23:45, David Zeuthen wrote:
> Hi,
> 
> Attached are the latest patches against GNOME VFS to use HAL for
> discovery of physical storage devices. There is one against HEAD and one
> against the stable 2.6 branch. It requires a working installation with
> HAL from CVS and the update-fstab.sh callout enabled; gnome-volume-
> manager is optional. For this patch you also need a recent version of
> GNOME VFS that doesn't crash when a name contains a '/' character.
> 
> By default the patch doesn't change anything; you will need to pass --
> enable-hal to configure to use it.

All right. I commited this to HEAD. Although i had to remove the #ifdef
USE_HAL around dont_use_hald to make it build without hal.

I don't have hal installed on my system, so i can't test this, but I
assume you guys tested it. The non-hal parts looks ok to me, and I
mostly glanced at the hal-specific parts which looked alright. 

One thing struck me though. In _hal_add_volume it seems like we're
always creating a drive for each volume. Thats not necessary in general.
Its perfectly fine for a volume to not have a drive. In fact, thats
expected behaviour for volumes that are not "removable media plugged
into a drive".

> I'd like to ask for this patch, pending review of course, to be included
> in the GNOME VFS tree. The patch is feature complete when dealing with
> physical storage device. Since HAL exposes a lot of information there
> are also a few enhancements - as such, this require discussion, see
> below and please comment.
> 
> 
> TODO/QUESTIONS/ISSUES
> 
>         Find a way to expose the HAL UDI of a drive and a volume such
> that e.g. Nautilus can extract more properties on the device

What about the attached (untested) gnome-vfs-export-hal-udi.patch? It
exports the hal udi as a string, whenever its availible. This lets
nautilus recover UDI information from computer:// since the Drive/Volume
id is availible in the desktop files there. This would make it easy to
do things like adding emblems in nautilus.

>         When libgphoto2 is completely halificated, get the GNOME VFS
> backend to work and add functionality (in this file) to show a camera
> icon that links to the appropriate GNOME VFS URI. (when the camera is
> not usb-storage based of course)
> 
>         Do the same for MP3 players when GNOME VFS backends for these
> emerge.

Are we totally sure we want to do this? Are these devices really generic
filesystems exposable in gnome-vfs? Can you save your word document on
the camera? Does it make sense?

I think its better to detect devices when plugged in, and perhaps launch
the right app, or maybe just show the device as plugged in somewhere
(notification area?). And then the application that handles cameras
should of course use hal and automatically do the right thing wrt its
user interface when the camera is plugged in. 

>         Please look at GNOME Bug 143888. Fixing this will allow a
> reasonable good desktop experience without automounting or autodetection
> of media.

I think John Palmeri is working on this.

>         When the patch is used GNOME VFS doesn't look for anything in
> /etc/fstab or /etc/mtab; this means that e.g. SMB or NFS network mounts
> are not shown in computer:///. Should this be changed?

I dunno really.

> ICONS
> 
> This patch by default uses a new set of icons as discussed in this
> thread on utopia-list
> 
>  http://mail.gnome.org/archives/utopia-list/2004-July/msg00009.html

Just make sure you get jimmac to draw the icons for you and put them in
gnome-icon-theme.

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's a notorious bohemian dog-catcher with a winning smile and a way with the 
ladies. She's a brilliant cat-loving mercenary from a different time and 
place. They fight crime! 

Index: libgnomevfs/GNOME_VFS_Daemon.idl
===================================================================
RCS file: /cvs/gnome/gnome-vfs/libgnomevfs/GNOME_VFS_Daemon.idl,v
retrieving revision 1.7
diff -u -p -r1.7 GNOME_VFS_Daemon.idl
--- libgnomevfs/GNOME_VFS_Daemon.idl	20 Apr 2004 13:27:46 -0000	1.7
+++ libgnomevfs/GNOME_VFS_Daemon.idl	9 Jul 2004 09:17:53 -0000
@@ -48,6 +48,7 @@ module GNOME {
 			string device_path;
 			long unix_device;
 			string gconf_id;
+		        string hal_udi;
 		};
 		typedef sequence<Volume> VolumeList;
 
@@ -61,6 +62,7 @@ module GNOME {
 			string icon;
 			boolean is_user_visible;
 			boolean is_connected;
+		        string hal_udi;
 		};
 		
 		typedef sequence<Drive> DriveList;
Index: libgnomevfs/gnome-vfs-drive.c
===================================================================
RCS file: /cvs/gnome/gnome-vfs/libgnomevfs/gnome-vfs-drive.c,v
retrieving revision 1.5
diff -u -p -r1.5 gnome-vfs-drive.c
--- libgnomevfs/gnome-vfs-drive.c	9 Jul 2004 08:39:16 -0000	1.5
+++ libgnomevfs/gnome-vfs-drive.c	9 Jul 2004 09:17:53 -0000
@@ -171,9 +171,7 @@ gnome_vfs_drive_finalize (GObject *objec
 	g_free (priv->activation_uri);
 	g_free (priv->display_name);
 	g_free (priv->icon);
-#ifdef USE_HAL
 	g_free (priv->hal_udi);
-#endif
 	g_free (priv);
 	drive->priv = NULL;
 	
@@ -254,6 +252,12 @@ gnome_vfs_drive_get_activation_uri (Gnom
 }
 
 char *
+gnome_vfs_drive_get_hal_udi (GnomeVFSDrive *drive)
+{
+	return g_strdup (drive->priv->hal_udi);
+}
+
+char *
 gnome_vfs_drive_get_display_name (GnomeVFSDrive *drive)
 {
 	return g_strdup (drive->priv->display_name);
@@ -340,6 +344,7 @@ gnome_vfs_drive_to_corba (GnomeVFSDrive 
 	corba_drive->activation_uri = corba_string_or_null_dup (drive->priv->activation_uri);
 	corba_drive->display_name = corba_string_or_null_dup (drive->priv->display_name);
 	corba_drive->icon = corba_string_or_null_dup (drive->priv->icon);
+	corba_drive->hal_udi = corba_string_or_null_dup (drive->priv->hal_udi);
 	
 	corba_drive->is_user_visible = drive->priv->is_user_visible;
 	corba_drive->is_connected = drive->priv->is_connected;
@@ -368,6 +373,7 @@ _gnome_vfs_drive_from_corba (const GNOME
 	drive->priv->activation_uri = decode_corba_string_or_null (corba_drive->activation_uri, TRUE);
 	drive->priv->display_name = decode_corba_string_or_null (corba_drive->display_name, TRUE);
 	drive->priv->icon = decode_corba_string_or_null (corba_drive->icon, TRUE);
+	drive->priv->hal_udi = decode_corba_string_or_null (corba_drive->hal_udi, TRUE);
 	
 	drive->priv->is_user_visible = corba_drive->is_user_visible;
 	drive->priv->is_connected = corba_drive->is_connected;
Index: libgnomevfs/gnome-vfs-drive.h
===================================================================
RCS file: /cvs/gnome/gnome-vfs/libgnomevfs/gnome-vfs-drive.h,v
retrieving revision 1.6
diff -u -p -r1.6 gnome-vfs-drive.h
--- libgnomevfs/gnome-vfs-drive.h	1 Mar 2004 09:43:31 -0000	1.6
+++ libgnomevfs/gnome-vfs-drive.h	9 Jul 2004 09:17:53 -0000
@@ -68,6 +68,7 @@ char *             gnome_vfs_drive_get_d
 char *             gnome_vfs_drive_get_activation_uri  (GnomeVFSDrive *drive);
 char *             gnome_vfs_drive_get_display_name    (GnomeVFSDrive *drive);
 char *             gnome_vfs_drive_get_icon            (GnomeVFSDrive *drive);
+char *             gnome_vfs_drive_get_hal_udi         (GnomeVFSDrive *drive);
 gboolean           gnome_vfs_drive_is_user_visible     (GnomeVFSDrive *drive);
 gboolean           gnome_vfs_drive_is_connected        (GnomeVFSDrive *drive);
 gboolean           gnome_vfs_drive_is_mounted          (GnomeVFSDrive *drive);
Index: libgnomevfs/gnome-vfs-hal-mounts.c
===================================================================
RCS file: /cvs/gnome/gnome-vfs/libgnomevfs/gnome-vfs-hal-mounts.c,v
retrieving revision 1.1
diff -u -p -r1.1 gnome-vfs-hal-mounts.c
--- libgnomevfs/gnome-vfs-hal-mounts.c	9 Jul 2004 08:39:16 -0000	1.1
+++ libgnomevfs/gnome-vfs-hal-mounts.c	9 Jul 2004 09:17:53 -0000
@@ -1064,11 +1064,11 @@ _hal_add_volume (GnomeVFSVolumeMonitorDa
 
 		drive->priv->hal_udi = g_strdup (
 #ifdef HAL_ONLY_SHOW_MOUNTED_VOLUMES
-			udi);
+			udi
 #else
-			hal_drive->no_partitions ? hal_drive->udi : udi);
+			hal_drive->no_partitions ? hal_drive->udi : udi
 #endif
-		
+		);
 		_gnome_vfs_volume_monitor_connected (volume_monitor, drive);
 		gnome_vfs_drive_unref (drive);
 	} 
@@ -1091,6 +1091,7 @@ _hal_add_volume (GnomeVFSVolumeMonitorDa
 			device_type = _hal_get_vol_type (hal_vol, hal_drive, hal_ctx);
 
 			vol = g_object_new (GNOME_VFS_TYPE_VOLUME, NULL);
+			vol->priv->hal_udi = g_strdup (udi);
 			vol->priv->volume_type = GNOME_VFS_VOLUME_TYPE_MOUNTPOINT;
 			
 			if (is_blank_disc) {
Index: libgnomevfs/gnome-vfs-volume-monitor-private.h
===================================================================
RCS file: /cvs/gnome/gnome-vfs/libgnomevfs/gnome-vfs-volume-monitor-private.h,v
retrieving revision 1.5
diff -u -p -r1.5 gnome-vfs-volume-monitor-private.h
--- libgnomevfs/gnome-vfs-volume-monitor-private.h	9 Jul 2004 08:39:16 -0000	1.5
+++ libgnomevfs/gnome-vfs-volume-monitor-private.h	9 Jul 2004 09:17:53 -0000
@@ -60,6 +60,9 @@ struct _GnomeVFSVolumePrivate {
 	char *device_path;
 	dev_t unix_device;
 
+	/* Only for HAL devices: */
+	char *hal_udi;
+
 	/* Only for connected servers */
 	char *gconf_id;
 
@@ -84,9 +87,8 @@ struct _GnomeVFSDrivePrivate {
 	gboolean is_user_visible;
 	gboolean is_connected;
 
-#ifdef USE_HAL
+	/* Only for HAL devices: */
 	char *hal_udi;
-#endif /* USE_HAL */
 };
 
 void _gnome_vfs_volume_set_drive                (GnomeVFSVolume        *volume,
Index: libgnomevfs/gnome-vfs-volume.c
===================================================================
RCS file: /cvs/gnome/gnome-vfs/libgnomevfs/gnome-vfs-volume.c,v
retrieving revision 1.5
diff -u -p -r1.5 gnome-vfs-volume.c
--- libgnomevfs/gnome-vfs-volume.c	9 Jul 2004 08:39:16 -0000	1.5
+++ libgnomevfs/gnome-vfs-volume.c	9 Jul 2004 09:17:53 -0000
@@ -209,6 +209,12 @@ gnome_vfs_volume_get_activation_uri (Gno
 }
 
 char *
+gnome_vfs_volume_get_hal_udi (GnomeVFSVolume *volume)
+{
+	return g_strdup (volume->priv->hal_udi);
+}
+
+char *
 gnome_vfs_volume_get_filesystem_type (GnomeVFSVolume *volume)
 {
 	return g_strdup (volume->priv->filesystem_type);
@@ -360,6 +366,7 @@ gnome_vfs_volume_to_corba (GnomeVFSVolum
 	corba_volume->display_name = corba_string_or_null_dup (volume->priv->display_name);
 	corba_volume->icon = corba_string_or_null_dup (volume->priv->icon);
 	corba_volume->gconf_id = corba_string_or_null_dup (volume->priv->gconf_id);
+	corba_volume->hal_udi = corba_string_or_null_dup (volume->priv->hal_udi);
 	
 	corba_volume->is_user_visible = volume->priv->is_user_visible;
 	corba_volume->is_read_only = volume->priv->is_read_only;
@@ -395,6 +402,7 @@ _gnome_vfs_volume_from_corba (const GNOM
 	volume->priv->display_name = decode_corba_string_or_null (corba_volume->display_name, TRUE);
 	volume->priv->icon = decode_corba_string_or_null (corba_volume->icon, TRUE);
 	volume->priv->gconf_id = decode_corba_string_or_null (corba_volume->gconf_id, TRUE);
+	volume->priv->hal_udi = decode_corba_string_or_null (corba_volume->hal_udi, TRUE);
 	
 	volume->priv->is_user_visible = corba_volume->is_user_visible;
 	volume->priv->is_read_only = corba_volume->is_read_only;
Index: libgnomevfs/gnome-vfs-volume.h
===================================================================
RCS file: /cvs/gnome/gnome-vfs/libgnomevfs/gnome-vfs-volume.h,v
retrieving revision 1.6
diff -u -p -r1.6 gnome-vfs-volume.h
--- libgnomevfs/gnome-vfs-volume.h	1 Mar 2004 09:43:31 -0000	1.6
+++ libgnomevfs/gnome-vfs-volume.h	9 Jul 2004 09:17:53 -0000
@@ -105,6 +105,7 @@ char *             gnome_vfs_volume_get_
 char *             gnome_vfs_volume_get_filesystem_type (GnomeVFSVolume *volume);
 char *             gnome_vfs_volume_get_display_name    (GnomeVFSVolume *volume);
 char *             gnome_vfs_volume_get_icon            (GnomeVFSVolume *volume);
+char *             gnome_vfs_volume_get_hal_udi         (GnomeVFSVolume *volume);
 gboolean           gnome_vfs_volume_is_user_visible     (GnomeVFSVolume *volume);
 gboolean           gnome_vfs_volume_is_read_only        (GnomeVFSVolume *volume);
 gboolean           gnome_vfs_volume_is_mounted          (GnomeVFSVolume *volume);