From: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
Alexander Ost <ost@ieee.org>, Git List <git@vger.kernel.org>
Subject: Re: [BUG] `make install' partly ignores `NO_INSTALL_HARDLINKS'
Date: Sat, 15 Aug 2020 08:15:56 +0700 [thread overview]
Message-ID: <20200815011544.GB12363@danh.dev> (raw)
In-Reply-To: <xmqqo8ndxhof.fsf@gitster.c.googlers.com>
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
next prev parent reply other threads:[~2020-08-15 21:53 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2020-08-17 16:40 ` Junio C Hamano
2020-08-17 17:06 ` Đoàn Trần Công Danh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200815011544.GB12363@danh.dev \
--to=congdanhqx@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=ost@ieee.org \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.