LVM2/tools/fsadm Makefile.in

LVM2/tools/fsadm Makefile.in

[meyering]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	meyering at sourceware.org	2007-10-03 09:46:58

Modified files:
	tools/fsadm    : Makefile.in 

Log message:
	Avoid link failure when building fsadm.
	
	* tools/fsadm/Makefile.in (LVMLIBS): Define.
	(fsadm): Link with $(LVMLIBS).
	
	Author: Nix <nix@esperi.org.uk>

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/fsadm/Makefile.in.diff?cvsroot=lvm2&r1=1.3&r2=1.4

--- LVM2/tools/fsadm/Makefile.in	2007/09/21 10:16:45	1.3
+++ LVM2/tools/fsadm/Makefile.in	2007/10/03 09:46:57	1.4
@@ -20,10 +20,12 @@
 
 TARGETS = fsadm
 
+LVMLIBS = -llvm
+
 include $(top_srcdir)/make.tmpl
 
 fsadm: $(OBJECTS)
-	$(CC) -o $@ $(CFLAGS) $(OBJECTS) -rdynamic
+	$(CC) -o $@ $(CFLAGS) $(OBJECTS) $(LDFLAGS) $(LVMLIBS) -rdynamic
 
 install: fsadm
 	$(INSTALL) -D $(OWNER) $(GROUP) -m 555 $(STRIP) fsadm \

Re: LVM2/tools/fsadm Makefile.in

[Alasdair G Kergon]

On Wed, Oct 03, 2007 at 09:46:58AM -0000, meyering at sourceware.org wrote:
> 	* tools/fsadm/Makefile.in (LVMLIBS): Define.
> 	(fsadm): Link with $(LVMLIBS).
 
>  fsadm: $(OBJECTS)
> -	$(CC) -o $@ $(CFLAGS) $(OBJECTS) -rdynamic
> +	$(CC) -o $@ $(CFLAGS) $(OBJECTS) $(LDFLAGS) $(LVMLIBS) -rdynamic
  
fsadm is meant to be independent of LVM2 so should not be using LVMLIBS.
What should be done to deal with this?

Alasdair
-- 
agk at redhat.com

Re: LVM2/tools/fsadm Makefile.in

[Bryn M. Reeves]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Alasdair G Kergon wrote:
> On Wed, Oct 03, 2007 at 09:46:58AM -0000, meyering at sourceware.org wrote:
>> 	* tools/fsadm/Makefile.in (LVMLIBS): Define.
>> 	(fsadm): Link with $(LVMLIBS).
>  
>>  fsadm: $(OBJECTS)
>> -	$(CC) -o $@ $(CFLAGS) $(OBJECTS) -rdynamic
>> +	$(CC) -o $@ $(CFLAGS) $(OBJECTS) $(LDFLAGS) $(LVMLIBS) -rdynamic
>   
> fsadm is meant to be independent of LVM2 so should not be using LVMLIBS.
> What should be done to deal with this?

It's just one call in the _usage function that was formerly a call to
basename. Eliminating basename usage in LVM2 is a good thing, but
perhaps this one should be left as-was until fsadm has a more complete
implementation?

Bryn.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFHA2js6YSQoMYUY94RAmR+AJ0QkCJl+lVIf6YHSJI+tUWFvNmtZgCgjTjR
Yz5WkTwgMh8ag/xZxXAdQps=
=WaXe
-----END PGP SIGNATURE-----

Re: LVM2/tools/fsadm Makefile.in

[Jim Meyering]

Alasdair G Kergon <agk@redhat.com> wrote:
> On Wed, Oct 03, 2007 at 09:46:58AM -0000, meyering at sourceware.org wrote:
>> 	* tools/fsadm/Makefile.in (LVMLIBS): Define.
>> 	(fsadm): Link with $(LVMLIBS).
>
>>  fsadm: $(OBJECTS)
>> -	$(CC) -o $@ $(CFLAGS) $(OBJECTS) -rdynamic
>> +	$(CC) -o $@ $(CFLAGS) $(OBJECTS) $(LDFLAGS) $(LVMLIBS) -rdynamic
>
> fsadm is meant to be independent of LVM2 so should not be using LVMLIBS.
> What should be done to deal with this?

Ok.
A few ways to do that:

    Move last_path_component from util.c to an independent header and
    make it a static inline function, then simply include it from fsadm.c.

    Add a Makefile rule that extracts the definition from util.c,
    and perhaps adding #include <string.h>,
    and making the function static inline.

    A couple more.  but I have to run.

Re: LVM2/tools/fsadm Makefile.in

[Jim Meyering]

Alasdair G Kergon <agk@redhat.com> wrote:
> On Wed, Oct 03, 2007 at 09:46:58AM -0000, meyering at sourceware.org wrote:
>> 	* tools/fsadm/Makefile.in (LVMLIBS): Define.
>> 	(fsadm): Link with $(LVMLIBS).
>
>>  fsadm: $(OBJECTS)
>> -	$(CC) -o $@ $(CFLAGS) $(OBJECTS) -rdynamic
>> +	$(CC) -o $@ $(CFLAGS) $(OBJECTS) $(LDFLAGS) $(LVMLIBS) -rdynamic
>
> fsadm is meant to be independent of LVM2 so should not be using LVMLIBS.
> What should be done to deal with this?

Unless I hear otherwise, I'll fix it by reverting the
Makefile.in change and applying this one instead:
-------------

Fix fsadm build failure a different way, so that fsadm doesn't use -llvm.

* lib/misc/util.c (last_path_component): Move definition to ...
* lib/misc/last-path-component.h (last_path_component): ...here.
New file.  Make the function "static inline".
* include/.symlinks: Add last-path-component.h.
* lib/misc/util.h (last_path_component): Remove declaration.
* tools/fsadm/fsadm.c: Include "last-path-component.h".
* tools/lvmcmdline.c: Likewise.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 include/.symlinks              |    1 +
 lib/misc/last-path-component.h |   27 +++++++++++++++++++++++++++
 lib/misc/util.c                |    7 +------
 lib/misc/util.h                |    2 --
 tools/fsadm/fsadm.c            |    1 +
 tools/lvmcmdline.c             |    1 +
 6 files changed, 31 insertions(+), 8 deletions(-)
 create mode 100644 lib/misc/last-path-component.h

diff --git a/include/.symlinks b/include/.symlinks
index a127e2c..7c54d9e 100644
--- a/include/.symlinks
+++ b/include/.symlinks
@@ -39,6 +39,7 @@
 ../lib/misc/crc.h
 ../lib/misc/intl.h
 ../lib/misc/util.h
+../lib/misc/last-path-component.h
 ../lib/misc/lib.h
 ../lib/misc/lvm-exec.h
 ../lib/misc/lvm-file.h
diff --git a/lib/misc/last-path-component.h b/lib/misc/last-path-component.h
new file mode 100644
index 0000000..69a1ce0
--- /dev/null
+++ b/lib/misc/last-path-component.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2007 Red Hat, Inc. All rights reserved.
+ *
+ * This file is part of LVM2.
+ *
+ * This copyrighted material is made available to anyone wishing to use,
+ * modify, copy, or redistribute it subject to the terms and conditions
+ * of the GNU Lesser General Public License v.2.1.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+/*
+ * Return the address of the last file name component of NAME.
+ * If NAME ends in a slash, return the empty string.
+ */
+
+#include <string.h>
+
+static inline char *last_path_component(char const *name)
+{
+	char const *slash = strrchr (name, '/');
+	char const *res = slash ? slash + 1 : name;
+	return (char *) res;
+}
diff --git a/lib/misc/util.c b/lib/misc/util.c
index b134c5e..75c7afa 100644
--- a/lib/misc/util.c
+++ b/lib/misc/util.c
@@ -19,9 +19,4 @@

 #include "lib.h"

-char *last_path_component(char const *name)
-{
-	char const *slash = strrchr (name, '/');
-	char const *res = slash ? slash + 1 : name;
-	return (char *) res;
-}
+/* empty for now.  */
diff --git a/lib/misc/util.h b/lib/misc/util.h
index 694864c..5e5677d 100644
--- a/lib/misc/util.h
+++ b/lib/misc/util.h
@@ -25,6 +25,4 @@
 		     (void) (&_a == &_b); \
 		     _a > _b ? _a : _b; })

-char *last_path_component(char const *name);
-
 #endif
diff --git a/tools/fsadm/fsadm.c b/tools/fsadm/fsadm.c
index 8621164..619228a 100644
--- a/tools/fsadm/fsadm.c
+++ b/tools/fsadm/fsadm.c
@@ -39,6 +39,7 @@
 #include <sys/vfs.h>

 #include "util.h"
+#include "last-path-component.h"

 #define log_error(str, x...) fprintf(stderr, "%s(%u):  " str "\n", __FILE__, __LINE__, x)

diff --git a/tools/lvmcmdline.c b/tools/lvmcmdline.c
index 35a1266..94575f3 100644
--- a/tools/lvmcmdline.c
+++ b/tools/lvmcmdline.c
@@ -20,6 +20,7 @@

 #include "stub.h"
 #include "lvm2cmd.h"
+#include "last-path-component.h"

 #include <signal.h>
 #include <syslog.h>
--
1.5.3.3.131.g34c6d

LVM2/tools/fsadm Makefile.in

[meyering]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	meyering at sourceware.org	2007-10-03 16:08:18

Modified files:
	tools/fsadm    : Makefile.in 

Log message:
	Revert last change.  fsadm must not depend on -llvm.
	
	Author: Jim Meyering <meyering@redhat.com>

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/fsadm/Makefile.in.diff?cvsroot=lvm2&r1=1.4&r2=1.5

--- LVM2/tools/fsadm/Makefile.in	2007/10/03 09:46:57	1.4
+++ LVM2/tools/fsadm/Makefile.in	2007/10/03 16:08:18	1.5
@@ -20,12 +20,10 @@
 
 TARGETS = fsadm
 
-LVMLIBS = -llvm
-
 include $(top_srcdir)/make.tmpl
 
 fsadm: $(OBJECTS)
-	$(CC) -o $@ $(CFLAGS) $(OBJECTS) $(LDFLAGS) $(LVMLIBS) -rdynamic
+	$(CC) -o $@ $(CFLAGS) $(OBJECTS) -rdynamic
 
 install: fsadm
 	$(INSTALL) -D $(OWNER) $(GROUP) -m 555 $(STRIP) fsadm \

Re: LVM2/tools/fsadm Makefile.in

[Alasdair G Kergon]

Yes, that's a better workaround for now, but still not satisfactory in
the long term - inline functions are awkward to debug with current tools
so we try to avoid using them.

Alasdair
-- 
agk at redhat.com