All of lore.kernel.org
 help / color / mirror / Atom feed
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: Tue, 18 Aug 2020 00:06:07 +0700	[thread overview]
Message-ID: <20200817170607.GA16630@danh.dev> (raw)
In-Reply-To: <xmqq8sed445k.fsf@gitster.c.googlers.com>

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

      reply	other threads:[~2020-08-17 17:06 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
2020-08-17 16:40           ` Junio C Hamano
2020-08-17 17:06             ` Đoàn Trần Công Danh [this message]

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=20200817170607.GA16630@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.