Use `ln -n` to prevent forming a symlink cycle, instead of rm'ing the source

[thakis]

Summary

This is a better fix for the problem fixed in r334972.

Also remove the rm'ing of the symlink destination that was there to
clean up the bots -- it's over a year later, bots should be happy now.

[rnk]

lgtm

I think @zturner wanted to teach lit to completely remove the Output/ directory for every test so we don't have to deal with stale files from previous tests hanging around. I remember objecting that we can't do that on startup since it's slow and not parallelized. However, I think we could probably add it as an early step to TestRunner.executeShTest so we don't have to add these extra 'rm' commands anymore.

[aganea]

In D64301#1574304, @rnk wrote:

I think @zturner wanted to teach lit to completely remove the Output/ directory for every test so we don't have to deal with stale files from previous tests hanging around. I remember objecting that we can't do that on startup since it's slow and not parallelized. However, I think we could probably add it as an early step to TestRunner.executeShTest so we don't have to add these extra 'rm' commands anymore.

Just a reminder, the DeleteFile API, and thus rm on Windows are async. There's no guarantee the files were deleted once rm completes. See this. Occasionally, the tests are failing locally on my PC because of that (maybe 1 out of 50 times).

[aganea]

This comment was removed by aganea.

[hubert.reinterpretcast]

ln -n is not a standard option that is portably supported. I'll be restoring the use of rm (with -f instead of -rf) and the associated comment for removing %t.fake.

[thakis]

In D64301#1602660, @hubert.reinterpretcast wrote:

ln -n is not a standard option that is portably supported. I'll be restoring the use of rm (with -f instead of -rf) and the associated comment for removing %t.fake.

Which platforms doesn't it work on?

[hubert.reinterpretcast]

In D64301#1603499, @thakis wrote:

Which platforms doesn't it work on?

ln -n means something different on AIX: https://www.ibm.com/support/knowledgecenter/en/ssw_aix_72/l_commands/ln.html
-n is not documented as a standard option for the ln utility by POSIX.1-2017: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ln.html

[thakis]

Thanks for sharing! (And for the fix, and apologies for breaking you.)

cfe/trunk/test/Driver/no-canonical-prefixes.c

 // Due to ln -sf:
 // REQUIRES: shell
-// RUN: rm -rf %t.real
 // RUN: mkdir -p %t.real
 // RUN: cd %t.real
 // RUN: ln -sf %clang test-clang
 // RUN: cd ..
-// Important to remove %t.fake: If it already is a symlink to %t.real when
-// `ln -sf %t.real %t.fake` runs, then that would symlink %t.real to itself,
-// forming a cycle.
-// RUN: rm -rf %t.fake
-// RUN: ln -sf %t.real %t.fake
+// If %.fake already is a symlink to %t.real when `ln -sf %t.real %t.fake`
+// runs, then that would symlink %t.real to itself, forming a cycle.
+// The `-n` flag prevents this.
+// RUN: ln -sfn %t.real %t.fake
 // RUN: cd %t.fake
 // RUN: ./test-clang -v -S %s 2>&1 | FileCheck --check-prefix=CANONICAL %s
 // RUN: ./test-clang -v -S %s -no-canonical-prefixes 2>&1 | FileCheck --check-prefix=NON-CANONICAL %s
 //
 // FIXME: This should really be '.real'.
 // CANONICAL: InstalledDir: {{.*}}.fake
 // CANONICAL: {{[/|\\]*}}clang{{.*}}" -cc1
 //
 // NON-CANONICAL: InstalledDir: .{{$}}
 // NON-CANONICAL: test-clang" -cc1