[(Apple,Git),09/13] Use symbolic links rather than hard links for files in libexec
diff mbox series

Message ID 20190129193818.8645-10-jeremyhu@apple.com
State New
Headers show
Series
  • Differences between git-2.20.1 and Apple Git-116
Related show

Commit Message

Jeremy Sequoia Jan. 29, 2019, 7:38 p.m. UTC
See <rdar://problem/10573201>

Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
---
 Makefile | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

brian m. carlson Jan. 30, 2019, 9:50 a.m. UTC | #1
On Tue, Jan 29, 2019 at 11:38:15AM -0800, Jeremy Huddleston Sequoia wrote:
> See <rdar://problem/10573201>

It's my understanding that Radars aren't public. Could you summarize the
reasons behind this change in the commit message for those of us who
don't have access to view this issue?
Jeremy Sequoia Jan. 30, 2019, 11:41 a.m. UTC | #2
> On Jan 30, 2019, at 01:50, brian m. carlson <sandals@crustytoothpaste.net> wrote:
> 
> On Tue, Jan 29, 2019 at 11:38:15AM -0800, Jeremy Huddleston Sequoia wrote:
>> See <rdar://problem/10573201>
> 
> It's my understanding that Radars aren't public. Could you summarize the
> reasons behind this change in the commit message for those of us who
> don't have access to view this issue?

There was a bug in some tool in our packaging pipeline that resulted in hardlinks not being preserved.  That was fixed, but I decided to leave these as symlinks anyways in case users did a file operation on Xcode.app that didn't preserve hard links.

The point here is that it would probably be nice to have hard vs soft be a configuration option.
Johannes Schindelin Jan. 30, 2019, 7:15 p.m. UTC | #3
Hi Jeremy,

On Wed, 30 Jan 2019, Jeremy Huddleston Sequoia wrote:

> > On Jan 30, 2019, at 01:50, brian m. carlson
> > <sandals@crustytoothpaste.net> wrote:
> > 
> > On Tue, Jan 29, 2019 at 11:38:15AM -0800, Jeremy Huddleston Sequoia wrote:
> >> See <rdar://problem/10573201>
> > 
> > It's my understanding that Radars aren't public. Could you summarize the
> > reasons behind this change in the commit message for those of us who
> > don't have access to view this issue?
> 
> There was a bug in some tool in our packaging pipeline that resulted in
> hardlinks not being preserved.  That was fixed, but I decided to leave
> these as symlinks anyways in case users did a file operation on
> Xcode.app that didn't preserve hard links.
> 
> The point here is that it would probably be nice to have hard vs soft be
> a configuration option.

Your patch does not make it a configuration option. (Or a build option,
which would probably be the more appropriate thing to do here.)

You need not spend the time on this, though, as Ævar already did, in
ad874608d8c9 (Makefile: optionally symlink libexec/git-core binaries to
bin/git, 2018-03-13), which made it in v2.18.0 already. All you need to do
is to define INSTALL_SYMLINKS.

Ciao,
Johannes
Jeremy Sequoia Jan. 30, 2019, 8:52 p.m. UTC | #4
> On Jan 30, 2019, at 11:15, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> Hi Jeremy,
> 
> On Wed, 30 Jan 2019, Jeremy Huddleston Sequoia wrote:
> 
>>> On Jan 30, 2019, at 01:50, brian m. carlson
>>> <sandals@crustytoothpaste.net> wrote:
>>> 
>>> On Tue, Jan 29, 2019 at 11:38:15AM -0800, Jeremy Huddleston Sequoia wrote:
>>>> See <rdar://problem/10573201>
>>> 
>>> It's my understanding that Radars aren't public. Could you summarize the
>>> reasons behind this change in the commit message for those of us who
>>> don't have access to view this issue?
>> 
>> There was a bug in some tool in our packaging pipeline that resulted in
>> hardlinks not being preserved.  That was fixed, but I decided to leave
>> these as symlinks anyways in case users did a file operation on
>> Xcode.app that didn't preserve hard links.
>> 
>> The point here is that it would probably be nice to have hard vs soft be
>> a configuration option.
> 
> Your patch does not make it a configuration option. (Or a build option,
> which would probably be the more appropriate thing to do here.)
> 
> You need not spend the time on this, though, as Ævar already did, in
> ad874608d8c9 (Makefile: optionally symlink libexec/git-core binaries to
> bin/git, 2018-03-13), which made it in v2.18.0 already. All you need to do
> is to define INSTALL_SYMLINKS.

Oh great, thanks!  I missed that when doing the last rebase.

Patch
diff mbox series

diff --git a/Makefile b/Makefile
index 1a44c811aa..60711d6abe 100644
--- a/Makefile
+++ b/Makefile
@@ -2065,10 +2065,7 @@  version.sp version.s version.o: EXTRA_CPPFLAGS = \
 		git rev-parse -q --verify HEAD 2>/dev/null)"'
 
 $(BUILT_INS): git$X
-	$(QUIET_BUILT_IN)$(RM) $@ && \
-	ln $< $@ 2>/dev/null || \
-	ln -s $< $@ 2>/dev/null || \
-	cp $< $@
+	$(QUIET_BUILT_IN)ln -fs $< $@
 
 command-list.h: generate-cmdlist.sh command-list.txt
 
@@ -2387,7 +2384,6 @@  git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB)
 
 $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)
 	$(QUIET_LNCP)$(RM) $@ && \
-	ln $< $@ 2>/dev/null || \
 	ln -s $< $@ 2>/dev/null || \
 	cp $< $@