git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [BUG] `make install' partly ignores `NO_INSTALL_HARDLINKS'
@ 2020-08-12  8:15 Alexander Ost
  2020-08-13 21:45 ` Eric Sunshine
  0 siblings, 1 reply; 9+ messages in thread
From: Alexander Ost @ 2020-08-12  8:15 UTC (permalink / raw)
  To: git

Hello,

I tried to install git on a filesystem that does not support hard
links (specifically, on a Linux bind mount).

Despite installing with `make NO_INSTALL_HARDLINKS=1 install', the
installation process tries to create hard links, and the installation
fails (a quick workaround is `sed -i git-gui/Makefile -e "s/ln /ln -s
/g"').

Here's the bugreport:

vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)
./configure --prefix=/filesystem_without_hardlink_support
make
make NO_INSTALL_HARDLINKS=1 install

What did you expect to happen? (Expected behavior)
Successful installation.

What happened instead? (Actual behavior)
Installation fails with error
  ln: failed to create hard link
‘/filesystem_without_hardlink_support/libexec/git-core/git-citool’ =>
‘/filesystem_without_hardlink_support/libexec/git-core/git-gui’:
Operation not permitted

What's different between what you expected and what actually happened?
The makefile `git-gui/Makefile' tries to create hard links. This is
not expected when running `make NO_INSTALL_HARDLINKS=1 install'

Anything else you want to add:

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.

[System Info]
git version:
git version 2.27.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
uname: Linux 3.10.0-957.10.1.el7.x86_64 #1 SMP Mon Mar 18 15:06:45 UTC
2019 x86_64
compiler info: gnuc: 4.8
libc info: glibc: 2.17


[Enabled Hooks]
not run from a git repository - no hooks to show
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Thanks for checking this!

Best,
/alex

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] `make install' partly ignores `NO_INSTALL_HARDLINKS'
  2020-08-12  8:15 [BUG] `make install' partly ignores `NO_INSTALL_HARDLINKS' Alexander Ost
@ 2020-08-13 21:45 ` Eric Sunshine
  2020-08-13 21:52   ` Junio C Hamano
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Sunshine @ 2020-08-13 21:45 UTC (permalink / raw)
  To: Alexander Ost; +Cc: Git List

On Wed, Aug 12, 2020 at 4:16 AM Alexander Ost <ost@ieee.org> wrote:
> Despite installing with `make NO_INSTALL_HARDLINKS=1 install', the
> installation process tries to create hard links, and the installation
> fails (a quick workaround is `sed -i git-gui/Makefile -e "s/ln /ln -s
> /g"').
>
> make NO_INSTALL_HARDLINKS=1 install
> ln: failed to create hard link
> ‘/filesystem_without_hardlink_support/libexec/git-core/git-citool’ =>
> ‘/filesystem_without_hardlink_support/libexec/git-core/git-gui’:
> Operation not permitted

Indeed, it appears that the git-gui Makefile does not respect
NO_INSTALL_HARDLINKS. The git-gui project is maintained and developed
outside of the Git project (even though it gets bundled with Git), so
it has its own issue tracker. It would probably be best to re-submit
this bug report there:

https://github.com/prati0100/git-gui/issues

Thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] `make install' partly ignores `NO_INSTALL_HARDLINKS'
  2020-08-13 21:45 ` Eric Sunshine
@ 2020-08-13 21:52   ` Junio C Hamano
  2020-08-14  8:33     ` Alexander Ost
  2020-08-14 15:02     ` Đoàn Trần Công Danh
  0 siblings, 2 replies; 9+ messages in thread
From: Junio C Hamano @ 2020-08-13 21:52 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Alexander Ost, Git List

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Wed, Aug 12, 2020 at 4:16 AM Alexander Ost <ost@ieee.org> wrote:
>> Despite installing with `make NO_INSTALL_HARDLINKS=1 install', the
>> installation process tries to create hard links, and the installation
>> fails (a quick workaround is `sed -i git-gui/Makefile -e "s/ln /ln -s
>> /g"').
>>
>> make NO_INSTALL_HARDLINKS=1 install
>> ln: failed to create hard link
>> ‘/filesystem_without_hardlink_support/libexec/git-core/git-citool’ =>
>> ‘/filesystem_without_hardlink_support/libexec/git-core/git-gui’:
>> Operation not permitted
>
> Indeed, it appears that the git-gui Makefile does not respect
> NO_INSTALL_HARDLINKS. The git-gui project is maintained and developed
> outside of the Git project (even though it gets bundled with Git), so
> it has its own issue tracker. It would probably be best to re-submit
> this bug report there:
>
> https://github.com/prati0100/git-gui/issues

Thanks.  Perhaps something along this line (which is totally
untested), as the top-level Makefile already exports
NO_INSTALL_HARDLINKS to submakes?



 Makefile | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index f10caedaa7..1cdbf8e504 100644
--- a/Makefile
+++ b/Makefile
@@ -44,6 +44,11 @@ endif
 ifndef INSTALL
 	INSTALL = install
 endif
+ifdef NO_INSTALL_HARDLINKS
+	LN = cp
+else
+	LN = ln
+endif
 
 RM_RF     ?= rm -rf
 RMDIR     ?= rmdir
@@ -57,7 +62,7 @@ INSTALL_X1 =
 INSTALL_A0 = find # space is required here
 INSTALL_A1 = | cpio -pud
 INSTALL_L0 = rm -f # space is required here
-INSTALL_L1 = && ln # space is required here
+INSTALL_L1 = && $(LN) # space is required here
 INSTALL_L2 =
 INSTALL_L3 =
 
@@ -87,7 +92,7 @@ ifndef V
 	INSTALL_L0 = dst=
 	INSTALL_L1 = && src=
 	INSTALL_L2 = && dst=
-	INSTALL_L3 = && echo '   ' 'LINK       ' `basename "$$dst"` '->' `basename "$$src"` && rm -f "$$dst" && ln "$$src" "$$dst"
+	INSTALL_L3 = && echo '   ' 'LINK       ' `basename "$$dst"` '->' `basename "$$src"` && rm -f "$$dst" && $(LN) "$$src" "$$dst"
 
 	CLEAN_DST = echo ' ' UNINSTALL
 	REMOVE_D0 = dir=

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] `make install' partly ignores `NO_INSTALL_HARDLINKS'
  2020-08-13 21:52   ` Junio C Hamano
@ 2020-08-14  8:33     ` Alexander Ost
  2020-08-14 15:02     ` Đoàn Trần Công Danh
  1 sibling, 0 replies; 9+ messages in thread
From: Alexander Ost @ 2020-08-14  8:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Sunshine, Git List

Thanks, I'll check it out.

Created a git-gui bug report (#41,
https://github.com/prati0100/git-gui/issues/41).

On Thu, Aug 13, 2020 at 11:52 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
> > On Wed, Aug 12, 2020 at 4:16 AM Alexander Ost <ost@ieee.org> wrote:
> >> Despite installing with `make NO_INSTALL_HARDLINKS=1 install', the
> >> installation process tries to create hard links, and the installation
> >> fails (a quick workaround is `sed -i git-gui/Makefile -e "s/ln /ln -s
> >> /g"').
> >>
> >> make NO_INSTALL_HARDLINKS=1 install
> >> ln: failed to create hard link
> >> ‘/filesystem_without_hardlink_support/libexec/git-core/git-citool’ =>
> >> ‘/filesystem_without_hardlink_support/libexec/git-core/git-gui’:
> >> Operation not permitted
> >
> > Indeed, it appears that the git-gui Makefile does not respect
> > NO_INSTALL_HARDLINKS. The git-gui project is maintained and developed
> > outside of the Git project (even though it gets bundled with Git), so
> > it has its own issue tracker. It would probably be best to re-submit
> > this bug report there:
> >
> > https://github.com/prati0100/git-gui/issues
>
> Thanks.  Perhaps something along this line (which is totally
> untested), as the top-level Makefile already exports
> NO_INSTALL_HARDLINKS to submakes?
>
>
>
>  Makefile | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f10caedaa7..1cdbf8e504 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -44,6 +44,11 @@ endif
>  ifndef INSTALL
>         INSTALL = install
>  endif
> +ifdef NO_INSTALL_HARDLINKS
> +       LN = cp
> +else
> +       LN = ln
> +endif
>
>  RM_RF     ?= rm -rf
>  RMDIR     ?= rmdir
> @@ -57,7 +62,7 @@ INSTALL_X1 =
>  INSTALL_A0 = find # space is required here
>  INSTALL_A1 = | cpio -pud
>  INSTALL_L0 = rm -f # space is required here
> -INSTALL_L1 = && ln # space is required here
> +INSTALL_L1 = && $(LN) # space is required here
>  INSTALL_L2 =
>  INSTALL_L3 =
>
> @@ -87,7 +92,7 @@ ifndef V
>         INSTALL_L0 = dst=
>         INSTALL_L1 = && src=
>         INSTALL_L2 = && dst=
> -       INSTALL_L3 = && echo '   ' 'LINK       ' `basename "$$dst"` '->' `basename "$$src"` && rm -f "$$dst" && ln "$$src" "$$dst"
> +       INSTALL_L3 = && echo '   ' 'LINK       ' `basename "$$dst"` '->' `basename "$$src"` && rm -f "$$dst" && $(LN) "$$src" "$$dst"
>
>         CLEAN_DST = echo ' ' UNINSTALL
>         REMOVE_D0 = dir=



-- 
-- 
Dr. Alexander Ost            | email: ost@ieee.org
Performance Consulting       | fon:   +49 (0)241 51859-230
Jahnstr. 11 / D-52066 Aachen | mobil: +49 (0)178 5431550

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] `make install' partly ignores `NO_INSTALL_HARDLINKS'
  2020-08-13 21:52   ` Junio C Hamano
  2020-08-14  8:33     ` Alexander Ost
@ 2020-08-14 15:02     ` Đoàn Trần Công Danh
  2020-08-14 17:26       ` Junio C Hamano
  1 sibling, 1 reply; 9+ messages in thread
From: Đoàn Trần Công Danh @ 2020-08-14 15:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Sunshine, Alexander Ost, Git List

On 2020-08-13 14:52:33-0700, Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > On Wed, Aug 12, 2020 at 4:16 AM Alexander Ost <ost@ieee.org> wrote:
> >> Despite installing with `make NO_INSTALL_HARDLINKS=1 install', the
> >> installation process tries to create hard links, and the installation
> >> fails (a quick workaround is `sed -i git-gui/Makefile -e "s/ln /ln -s
> >> /g"').
> >>
> >> make NO_INSTALL_HARDLINKS=1 install
> >> ln: failed to create hard link
> >> ‘/filesystem_without_hardlink_support/libexec/git-core/git-citool’ =>
> >> ‘/filesystem_without_hardlink_support/libexec/git-core/git-gui’:
> >> Operation not permitted
> >
> > Indeed, it appears that the git-gui Makefile does not respect
> > NO_INSTALL_HARDLINKS. The git-gui project is maintained and developed
> > outside of the Git project (even though it gets bundled with Git), so
> > it has its own issue tracker. It would probably be best to re-submit
> > this bug report there:
> >
> > https://github.com/prati0100/git-gui/issues
> 
> Thanks.  Perhaps something along this line (which is totally
> untested), as the top-level Makefile already exports
> NO_INSTALL_HARDLINKS to submakes?
> 
> 
> 
>  Makefile | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index f10caedaa7..1cdbf8e504 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -44,6 +44,11 @@ endif
>  ifndef INSTALL
>  	INSTALL = install
>  endif
> +ifdef NO_INSTALL_HARDLINKS
> +	LN = cp

Since both git-citool and git-gui will be installed into same
directory "$(libexecdir)", I think it would make more sense to use:

	LN = ln -s

here instead?

> +else
> +	LN = ln
> +endif
>  
>  RM_RF     ?= rm -rf
>  RMDIR     ?= rmdir
> @@ -57,7 +62,7 @@ INSTALL_X1 =
>  INSTALL_A0 = find # space is required here
>  INSTALL_A1 = | cpio -pud
>  INSTALL_L0 = rm -f # space is required here
> -INSTALL_L1 = && ln # space is required here
> +INSTALL_L1 = && $(LN) # space is required here
>  INSTALL_L2 =
>  INSTALL_L3 =
>  
> @@ -87,7 +92,7 @@ ifndef V
>  	INSTALL_L0 = dst=
>  	INSTALL_L1 = && src=
>  	INSTALL_L2 = && dst=
> -	INSTALL_L3 = && echo '   ' 'LINK       ' `basename "$$dst"` '->' `basename "$$src"` && rm -f "$$dst" && ln "$$src" "$$dst"
> +	INSTALL_L3 = && echo '   ' 'LINK       ' `basename "$$dst"` '->' `basename "$$src"` && rm -f "$$dst" && $(LN) "$$src" "$$dst"
>  
>  	CLEAN_DST = echo ' ' UNINSTALL
>  	REMOVE_D0 = dir=

-- 
Danh

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] `make install' partly ignores `NO_INSTALL_HARDLINKS'
  2020-08-14 15:02     ` Đoàn Trần Công Danh
@ 2020-08-14 17:26       ` Junio C Hamano
  2020-08-15  1:15         ` Đoàn Trần Công Danh
  0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2020-08-14 17:26 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: Eric Sunshine, Alexander Ost, Git List

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

>> Thanks.  Perhaps something along this line (which is totally
>> untested), as the top-level Makefile already exports
>> NO_INSTALL_HARDLINKS to submakes?
>> 
>> 
>> 
>>  Makefile | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Makefile b/Makefile
>> index f10caedaa7..1cdbf8e504 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -44,6 +44,11 @@ endif
>>  ifndef INSTALL
>>  	INSTALL = install
>>  endif
>> +ifdef NO_INSTALL_HARDLINKS
>> +	LN = cp
>
> Since both git-citool and git-gui will be installed into same
> directory "$(libexecdir)", I think it would make more sense to use:
>
> 	LN = ln -s
>
> here instead?

In the top-level Makefile, INSTALL_SYMLINKS make macro does exist,
but it is not exported to submakes.  If it were, something like

    ifdef INSTALL_SYMLINKS
	LN = ln -s
    else
    ifdef NO_INSTALL_HARDLINKS
	LN = cp
    else
	LN = ln
    endif
    endif

might become possible, but you'd need to audit what is fed to $(LN)
at the locations the macro is used and make necessary adjustment
accordingly.  "cp A ../B" or "ln A ../B" will make a usable copy of
file A appear inside ../B directory, but "ln -s A ../B" will not,
and I didn't see if all uses of $(LN) was to give synonyms to what
is already installed, or some of them were truly installing from the
build location when I gave the "something along this line" example.




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] `make install' partly ignores `NO_INSTALL_HARDLINKS'
  2020-08-14 17:26       ` Junio C Hamano
@ 2020-08-15  1:15         ` Đoàn Trần Công Danh
  2020-08-17 16:40           ` Junio C Hamano
  0 siblings, 1 reply; 9+ messages in thread
From: Đoàn Trần Công Danh @ 2020-08-15  1:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Sunshine, Alexander Ost, Git List

On 2020-08-14 10:26:24-0700, Junio C Hamano <gitster@pobox.com> wrote:
> > Since both git-citool and git-gui will be installed into same
> > directory "$(libexecdir)", I think it would make more sense to use:
> >
> > 	LN = ln -s
> >
> > here instead?
> 
> In the top-level Makefile, INSTALL_SYMLINKS make macro does exist,
> but it is not exported to submakes.  If it were, something like
> 
>     ifdef INSTALL_SYMLINKS
> 	LN = ln -s
>     else
>     ifdef NO_INSTALL_HARDLINKS
> 	LN = cp
>     else
> 	LN = ln
>     endif
>     endif
> 
> might become possible, but you'd need to audit what is fed to $(LN)
> at the locations the macro is used and make necessary adjustment
> accordingly.  "cp A ../B" or "ln A ../B" will make a usable copy of
> file A appear inside ../B directory, but "ln -s A ../B" will not,
> and I didn't see if all uses of $(LN) was to give synonyms to what
> is already installed, or some of them were truly installing from the
> build location when I gave the "something along this line" example.

Yes, the top-level Makefile seems to have a special branch for
BUILT_INS, in which, we will create symlink for those builtin in
libexecdr if NO_INSTALL_HARDLINKS is defined.

I was aiming for something like this to make git-gui a bit more
consistent with top-level Git, with or without INSTALL_SYMLINKS
exported:

----------8<--------
diff --git a/git-gui/Makefile b/git-gui/Makefile
index f10caedaa7..1a7d4b5075 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -48,6 +48,13 @@ endif
 RM_RF     ?= rm -rf
 RMDIR     ?= rmdir
 
+ifneq (,$(NO_INSTALL_HARDLINKS)$(INSTALL_SYMLINKS))
+	SAMEDIR_LN = ln -s
+else
+	SAMEDIR_LN = ln
+endif
+
+
 INSTALL_D0 = $(INSTALL) -d -m 755 # space is required here
 INSTALL_D1 =
 INSTALL_R0 = $(INSTALL) -m 644 # space is required here
@@ -57,7 +64,7 @@ INSTALL_X1 =
 INSTALL_A0 = find # space is required here
 INSTALL_A1 = | cpio -pud
 INSTALL_L0 = rm -f # space is required here
-INSTALL_L1 = && ln # space is required here
+INSTALL_L1 = && $(SAMEDIR_LN) # space is required here
 INSTALL_L2 =
 INSTALL_L3 =
 
@@ -87,7 +94,7 @@ ifndef V
 	INSTALL_L0 = dst=
 	INSTALL_L1 = && src=
 	INSTALL_L2 = && dst=
-	INSTALL_L3 = && echo '   ' 'LINK       ' `basename "$$dst"` '->' `basename "$$src"` && rm -f "$$dst" && ln "$$src" "$$dst"
+	INSTALL_L3 = && echo '   ' 'LINK       ' `basename "$$dst"` '->' `basename "$$src"` && rm -f "$$dst" && $(SAMEDIR_LN) "$$src" "$$dst"
 
 	CLEAN_DST = echo ' ' UNINSTALL
 	REMOVE_D0 = dir=
@@ -127,6 +134,12 @@ TCL_PATH_SQ = $(subst ','\'',$(TCL_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 TCLTK_PATH_SED = $(subst ','\'',$(subst \,\\,$(TCLTK_PATH)))
 
+ifneq (,$(NO_INSTALL_HARDLINKS)$(INSTALL_SYMLINKS))
+	gitexecdir_SQ_for_LN =
+else
+	gitexecdir_SQ_for_LN = $(DESTDIR_SQ)$(gitexecdir_SQ)/
+endif
+
 gg_libdir ?= $(sharedir)/git-gui/lib
 libdir_SQ  = $(subst ','\'',$(gg_libdir))
 libdir_SED = $(subst ','\'',$(subst \,\\,$(gg_libdir_sed_in)))
@@ -293,7 +306,7 @@ install: all
 	$(QUIET)$(INSTALL_D0)'$(DESTDIR_SQ)$(gitexecdir_SQ)' $(INSTALL_D1)
 	$(QUIET)$(INSTALL_X0)git-gui $(INSTALL_X1) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
 	$(QUIET)$(INSTALL_X0)git-gui--askpass $(INSTALL_X1) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
-	$(QUIET)$(foreach p,$(GITGUI_BUILT_INS), $(INSTALL_L0)'$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' $(INSTALL_L1)'$(DESTDIR_SQ)$(gitexecdir_SQ)/git-gui' $(INSTALL_L2)'$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' $(INSTALL_L3) &&) true
+	$(QUIET)$(foreach p,$(GITGUI_BUILT_INS), $(INSTALL_L0)'$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' $(INSTALL_L1)'$(gitexecdir_SQ_for_LN)git-gui' $(INSTALL_L2)'$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' $(INSTALL_L3) &&) true
 ifdef GITGUI_WINDOWS_WRAPPER
 	$(QUIET)$(INSTALL_R0)git-gui.tcl $(INSTALL_R1) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
 endif
-------->8-----------

Given that, we also tried to make symlinks when NO_INSTALL_SYMLINKS is
given, I think it's fine to assume that symlink should available on
those platform.

-- 
Danh

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] `make install' partly ignores `NO_INSTALL_HARDLINKS'
  2020-08-15  1:15         ` Đoàn Trần Công Danh
@ 2020-08-17 16:40           ` Junio C Hamano
  2020-08-17 17:06             ` Đoàn Trần Công Danh
  0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2020-08-17 16:40 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: Eric Sunshine, Alexander Ost, Git List

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> On 2020-08-14 10:26:24-0700, Junio C Hamano <gitster@pobox.com> wrote:
>> > Since both git-citool and git-gui will be installed into same
>> > directory "$(libexecdir)", I think it would make more sense to use:
>> >
>> > 	LN = ln -s
>> >
>> > here instead?
>> 
>> In the top-level Makefile, INSTALL_SYMLINKS make macro does exist,
>> but it is not exported to submakes.  If it were, something like
>> 
>>     ifdef INSTALL_SYMLINKS
>> 	LN = ln -s
>>     else
>>     ifdef NO_INSTALL_HARDLINKS
>> 	LN = cp
>>     else
>> 	LN = ln
>>     endif
>>     endif
>> 
>> might become possible, but you'd need to audit what is fed to $(LN)
>> at the locations the macro is used and make necessary adjustment
>> accordingly.  "cp A ../B" or "ln A ../B" will make a usable copy of
>> file A appear inside ../B directory, but "ln -s A ../B" will not,
>> and I didn't see if all uses of $(LN) was to give synonyms to what
>> is already installed, or some of them were truly installing from the
>> build location when I gave the "something along this line" example.
>
> Yes, the top-level Makefile seems to have a special branch for
> BUILT_INS, in which, we will create symlink for those builtin in
> libexecdr if NO_INSTALL_HARDLINKS is defined.

Did you mean pieces like this?

	for p in $(filter $(install_bindir_programs),$(BUILT_INS)); do \
		$(RM) "$$bindir/$$p" && \
		test -n "$(INSTALL_SYMLINKS)" && \
		ln -s "git$X" "$$bindir/$$p" || \
		{ test -z "$(NO_INSTALL_HARDLINKS)" && \
		  ln "$$bindir/git$X" "$$bindir/$$p" 2>/dev/null || \
		  ln -s "git$X" "$$bindir/$$p" 2>/dev/null || \
		  cp "$$bindir/git$X" "$$bindir/$$p" || exit; } \
	done && \

The symlinks happen ONLY when INSTALL_SYMLINKS is asked for.  Not
all filesystems support symbolic links, hardlinks never suffer from
dangling link problem, and often they are cheaper.

> I was aiming for something like this to make git-gui a bit more
> consistent with top-level Git, with or without INSTALL_SYMLINKS
> exported:

So with or without optional INSTALL_SYMLINKS exported, what I gave
you is what is the most consistent with the top-level, that is, if
INSTALL_SYMLINKS is there, we do "ln -s".  Otherwise, we check
NO_INSTALL_HARDLINKS and we do either "ln" or "cp".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] `make install' partly ignores `NO_INSTALL_HARDLINKS'
  2020-08-17 16:40           ` Junio C Hamano
@ 2020-08-17 17:06             ` Đoàn Trần Công Danh
  0 siblings, 0 replies; 9+ messages in thread
From: Đoàn Trần Công Danh @ 2020-08-17 17:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Sunshine, Alexander Ost, Git List

On 2020-08-17 09:40:23-0700, Junio C Hamano <gitster@pobox.com> wrote:
> Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:
> 
> > On 2020-08-14 10:26:24-0700, Junio C Hamano <gitster@pobox.com> wrote:
> >> > Since both git-citool and git-gui will be installed into same
> >> > directory "$(libexecdir)", I think it would make more sense to use:
> >> >
> >> > 	LN = ln -s
> >> >
> >> > here instead?
> >> 
> >> In the top-level Makefile, INSTALL_SYMLINKS make macro does exist,
> >> but it is not exported to submakes.  If it were, something like
> >> 
> >>     ifdef INSTALL_SYMLINKS
> >> 	LN = ln -s
> >>     else
> >>     ifdef NO_INSTALL_HARDLINKS
> >> 	LN = cp
> >>     else
> >> 	LN = ln
> >>     endif
> >>     endif
> >> 
> >> might become possible, but you'd need to audit what is fed to $(LN)
> >> at the locations the macro is used and make necessary adjustment
> >> accordingly.  "cp A ../B" or "ln A ../B" will make a usable copy of
> >> file A appear inside ../B directory, but "ln -s A ../B" will not,
> >> and I didn't see if all uses of $(LN) was to give synonyms to what
> >> is already installed, or some of them were truly installing from the
> >> build location when I gave the "something along this line" example.
> >
> > Yes, the top-level Makefile seems to have a special branch for
> > BUILT_INS, in which, we will create symlink for those builtin in
> > libexecdr if NO_INSTALL_HARDLINKS is defined.
> 
> Did you mean pieces like this?
> 
> 	for p in $(filter $(install_bindir_programs),$(BUILT_INS)); do \
> 		$(RM) "$$bindir/$$p" && \
> 		test -n "$(INSTALL_SYMLINKS)" && \
> 		ln -s "git$X" "$$bindir/$$p" || \
> 		{ test -z "$(NO_INSTALL_HARDLINKS)" && \
> 		  ln "$$bindir/git$X" "$$bindir/$$p" 2>/dev/null || \
> 		  ln -s "git$X" "$$bindir/$$p" 2>/dev/null || \
> 		  cp "$$bindir/git$X" "$$bindir/$$p" || exit; } \
> 	done && \

Yes, I meant this piece.

> The symlinks happen ONLY when INSTALL_SYMLINKS is asked for.

Not what I understand from that code.
When `INSTALL_SYMLINKS` is not asked, the shell will jump to this block:

 		{ test -z "$(NO_INSTALL_HARDLINKS)" && \
 		  ln "$$bindir/git$X" "$$bindir/$$p" 2>/dev/null || \
 		  ln -s "git$X" "$$bindir/$$p" 2>/dev/null || \
 		  cp "$$bindir/git$X" "$$bindir/$$p" || exit; } \

When NO_INSTALL_HARDLINKS is asked, the shell will jump to last
2 clauses:

 		  ln -s "git$X" "$$bindir/$$p" 2>/dev/null || \
 		  cp "$$bindir/git$X" "$$bindir/$$p" || exit; } \

shell try to "ln -s" first, when it failed to symlink, cp will be used.

In fact, this's what we used for packaging in distro, we often ask for:

	NO_INSTALL_HARDLINKS

> Not all filesystems support symbolic links,

This is correct, but most sane package manager will do the right thing.

> hardlinks never suffer from
> dangling link problem, and often they are cheaper.

Part of the reason we don't ask for INSTALL_SYMLINKS,
because with INSTALL_SYMLINKS, symlink will be make from
$(libexecdir)/git to $(bindir)/git.

But, NO_INSTALL_HARDLINKS will only symlink inside $(libexecdir), e.g:

	$ ls -al $(/usr/bin/git --exec-path )/git
	-rwxr-xr-x 1 root root 3193704 2020-08-12 21:37 /usr/libexec/git-core/git
	$ ls -al $(/usr/bin/git --exec-path )/git-ls-files
	lrwxrwxrwx 1 root root 3 2020-08-12 21:37 /usr/libexec/git-core/git-ls-files -> git

And those symlinks are packaged and managed by package manager,
so, no, no dangling problem.


-- 
Danh

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-08-17 17:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-12  8:15 [BUG] `make install' partly ignores `NO_INSTALL_HARDLINKS' Alexander Ost
2020-08-13 21:45 ` Eric Sunshine
2020-08-13 21:52   ` Junio C Hamano
2020-08-14  8:33     ` Alexander Ost
2020-08-14 15:02     ` Đoàn Trần Công Danh
2020-08-14 17:26       ` Junio C Hamano
2020-08-15  1:15         ` Đoàn Trần Công Danh
2020-08-17 16:40           ` Junio C Hamano
2020-08-17 17:06             ` Đoàn Trần Công Danh

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git