diff mbox series

xfsdump: don't fail installation if /sbin is symlink of /usr/sbin

Message ID 20181101104732.19322-1-jtulak@redhat.com (mailing list archive)
State New, archived
Headers show
Series xfsdump: don't fail installation if /sbin is symlink of /usr/sbin | expand

Commit Message

Jan Tulak Nov. 1, 2018, 10:47 a.m. UTC
Some distributions, like Fedora, have /bin and /sbin as symlinks
pointing to /usr/* and this patch adds compatibility for these cases.

Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
 dump/Makefile    | 4 +++-
 restore/Makefile | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

Eric Sandeen Dec. 5, 2018, 4:48 a.m. UTC | #1
On 11/1/18 5:47 AM, Jan Tulak wrote:
> Some distributions, like Fedora, have /bin and /sbin as symlinks
> pointing to /usr/* and this patch adds compatibility for these cases.

xfsdump builds fine on Fedora, but only because somebody (me) was
bad long long ago, in the spec file:

# Bit of a hack to move files from /sbin to /usr/sbin
(cd $RPM_BUILD_ROOT/%{_sbindir}; rm xfsdump xfsrestore)
(cd $RPM_BUILD_ROOT/%{_sbindir}; mv ../../sbin/xfsdump .)
(cd $RPM_BUILD_ROOT/%{_sbindir}; mv ../../sbin/xfsrestore .)

I ... don't know why that was done.  Seems like dump/restore /should/
just live in /sbin, no?  I'm not sure what the symlink is/was for
in the first place...

but I guess Debian does package it up that way:

# dpkg-query -L xfsdump | grep xfsrestore | xargs ls -l
-rwxr-xr-x 1 root root 246088 Aug  1  2014 /sbin/xfsrestore
lrwxrwxrwx 1 root root     16 Aug  1  2014 /usr/sbin/xfsrestore -> /sbin/xfsrestore

so, um, I guess this seems ok to handle the /sbin == /usr/sbin case.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Thanks,
-Eric

> Signed-off-by: Jan Tulak <jtulak@redhat.com>
> ---
>  dump/Makefile    | 4 +++-
>  restore/Makefile | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/dump/Makefile b/dump/Makefile
> index 97879fa..66f00d3 100644
> --- a/dump/Makefile
> +++ b/dump/Makefile
> @@ -100,7 +100,9 @@ install: default
>  	$(INSTALL) -m 755 -d $(PKG_ROOT_SBIN_DIR)
>  	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_ROOT_SBIN_DIR)
>  	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
> -	$(INSTALL) -S $(PKG_ROOT_SBIN_DIR)/$(LTCOMMAND) $(PKG_SBIN_DIR)/$(LTCOMMAND)
> +	# skip symlink when /sbin is alread symlinked to /usr/sbin, like on Fedora
> +	test $(PKG_ROOT_SBIN_DIR) -ef $(PKG_SBIN_DIR) || \
> +		$(INSTALL) -S $(PKG_ROOT_SBIN_DIR)/$(LTCOMMAND) $(PKG_SBIN_DIR)/$(LTCOMMAND)
>  install-dev:
>  
>  .dep: $(COMMINCL) $(COMMON) $(INVINCL) $(INVCOMMON)
> diff --git a/restore/Makefile b/restore/Makefile
> index c6f3f25..20c870a 100644
> --- a/restore/Makefile
> +++ b/restore/Makefile
> @@ -110,7 +110,9 @@ install: default
>  	$(INSTALL) -m 755 -d $(PKG_ROOT_SBIN_DIR)
>  	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_ROOT_SBIN_DIR)
>  	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
> -	$(INSTALL) -S $(PKG_ROOT_SBIN_DIR)/$(LTCOMMAND) $(PKG_SBIN_DIR)/$(LTCOMMAND)
> +	# skip symlink when /sbin is alread symlinked to /usr/sbin, like on Fedora
> +	test $(PKG_ROOT_SBIN_DIR) -ef $(PKG_SBIN_DIR) || \
> +		$(INSTALL) -S $(PKG_ROOT_SBIN_DIR)/$(LTCOMMAND) $(PKG_SBIN_DIR)/$(LTCOMMAND)
>  install-dev:
>  
>  .dep: $(COMMINCL) $(COMMON) $(INVINCL) $(INVCOMMON)
>
Eric Sandeen Dec. 5, 2018, 4:56 a.m. UTC | #2
On 12/4/18 10:48 PM, Eric Sandeen wrote:
> On 11/1/18 5:47 AM, Jan Tulak wrote:
>> Some distributions, like Fedora, have /bin and /sbin as symlinks
>> pointing to /usr/* and this patch adds compatibility for these cases.
> 
> xfsdump builds fine on Fedora, but only because somebody (me) was
> bad long long ago, in the spec file:
> 
> # Bit of a hack to move files from /sbin to /usr/sbin
> (cd $RPM_BUILD_ROOT/%{_sbindir}; rm xfsdump xfsrestore)
> (cd $RPM_BUILD_ROOT/%{_sbindir}; mv ../../sbin/xfsdump .)
> (cd $RPM_BUILD_ROOT/%{_sbindir}; mv ../../sbin/xfsrestore .)
> 
> I ... don't know why that was done.  Seems like dump/restore /should/
> just live in /sbin, no?  I'm not sure what the symlink is/was for
> in the first place...
> 
> but I guess Debian does package it up that way:
> 
> # dpkg-query -L xfsdump | grep xfsrestore | xargs ls -l
> -rwxr-xr-x 1 root root 246088 Aug  1  2014 /sbin/xfsrestore
> lrwxrwxrwx 1 root root     16 Aug  1  2014 /usr/sbin/xfsrestore -> /sbin/xfsrestore
> 
> so, um, I guess this seems ok to handle the /sbin == /usr/sbin case.
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Heh, ok, yes your fix is what I should have done 6 years ago :D

Thanks,
-Eric
Jan Tulak Jan. 9, 2019, 1:53 p.m. UTC | #3
On Wed, Dec 5, 2018 at 5:56 AM Eric Sandeen <sandeen@sandeen.net> wrote:
>
> On 12/4/18 10:48 PM, Eric Sandeen wrote:
> > On 11/1/18 5:47 AM, Jan Tulak wrote:

snip

> >
> > so, um, I guess this seems ok to handle the /sbin == /usr/sbin case.
> >
> > Reviewed-by: Eric Sandeen <sandeen@redhat.com>
>
> Heh, ok, yes your fix is what I should have done 6 years ago :D
>
> Thanks,
> -Eric

This is not merged yet. Maybe it just got off your list somehow? :-)

Cheers
Jan
Eric Sandeen Jan. 9, 2019, 3 p.m. UTC | #4
On 1/9/19 7:53 AM, Jan Tulak wrote:
> On Wed, Dec 5, 2018 at 5:56 AM Eric Sandeen <sandeen@sandeen.net> wrote:
>>
>> On 12/4/18 10:48 PM, Eric Sandeen wrote:
>>> On 11/1/18 5:47 AM, Jan Tulak wrote:
> 
> snip
> 
>>>
>>> so, um, I guess this seems ok to handle the /sbin == /usr/sbin case.
>>>
>>> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
>>
>> Heh, ok, yes your fix is what I should have done 6 years ago :D
>>
>> Thanks,
>> -Eric
> 
> This is not merged yet. Maybe it just got off your list somehow? :-)

Yep, it fell through the cracks, thanks for the reminder.

-Eric
diff mbox series

Patch

diff --git a/dump/Makefile b/dump/Makefile
index 97879fa..66f00d3 100644
--- a/dump/Makefile
+++ b/dump/Makefile
@@ -100,7 +100,9 @@  install: default
 	$(INSTALL) -m 755 -d $(PKG_ROOT_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_ROOT_SBIN_DIR)
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
-	$(INSTALL) -S $(PKG_ROOT_SBIN_DIR)/$(LTCOMMAND) $(PKG_SBIN_DIR)/$(LTCOMMAND)
+	# skip symlink when /sbin is alread symlinked to /usr/sbin, like on Fedora
+	test $(PKG_ROOT_SBIN_DIR) -ef $(PKG_SBIN_DIR) || \
+		$(INSTALL) -S $(PKG_ROOT_SBIN_DIR)/$(LTCOMMAND) $(PKG_SBIN_DIR)/$(LTCOMMAND)
 install-dev:
 
 .dep: $(COMMINCL) $(COMMON) $(INVINCL) $(INVCOMMON)
diff --git a/restore/Makefile b/restore/Makefile
index c6f3f25..20c870a 100644
--- a/restore/Makefile
+++ b/restore/Makefile
@@ -110,7 +110,9 @@  install: default
 	$(INSTALL) -m 755 -d $(PKG_ROOT_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_ROOT_SBIN_DIR)
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
-	$(INSTALL) -S $(PKG_ROOT_SBIN_DIR)/$(LTCOMMAND) $(PKG_SBIN_DIR)/$(LTCOMMAND)
+	# skip symlink when /sbin is alread symlinked to /usr/sbin, like on Fedora
+	test $(PKG_ROOT_SBIN_DIR) -ef $(PKG_SBIN_DIR) || \
+		$(INSTALL) -S $(PKG_ROOT_SBIN_DIR)/$(LTCOMMAND) $(PKG_SBIN_DIR)/$(LTCOMMAND)
 install-dev:
 
 .dep: $(COMMINCL) $(COMMON) $(INVINCL) $(INVCOMMON)