This also fixes a few remaining inconsistencies in the symlink behavior on
Linux which were caused by an incorrect implementation of the procedure that
turns paths from the command line into subpaths relative to the repository
root. Several tests have been adapted, extended, and limitations wrt Windows
removed. Also contained are some cleanup patches that remove or replace
obsolete path handling procedures.

16 patches for repository http://darcs.net/screened:

patch f4ccc207d417f04bab9aa28639e79f18c8b4405d
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Apr 30 10:25:09 CEST 2022
  * remove Darcs.Util.Compat.canonFilename

  It's (few) uses were replaced with either makeAbsolute or canonicalizePath
  from System.Directory.

patch 107e72aac2d2bdb08a7af158ed7081daa33c9220
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue May 10 01:54:42 CEST 2022
  * add two clarifying comments

patch 18b39d12754b7be25e2d0511c3b3c6792370ebf4
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun May 22 18:21:40 CEST 2022
  * require unix-compat >= 0.6 for improved symlink support

patch c30a57ae84c02fe0fd86093d25945ce1f8ef0ac2
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu May 12 12:10:51 CEST 2022
  * fix all symlink problems, including Windows

patch ad238719c7c8a09af6c2470fe89591f77bcd38ca
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri May 13 12:47:10 CEST 2022
  * Darcs.Util.Path: add missing HasCallStack constraints

patch 32b6aef841433617d2b4610cb8dbf78578dd2f6d
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri May 13 12:41:55 CEST 2022
  * make errors in Darcs.Util.Path.decodeWhite explicit

  This means we no longer trust that path names in patches have valid
  white-space encoding. Instead this is now a proper parse error.

patch 9dc3e1c63ab3675f0d70809f5cbb0bd32f847e63
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri May 13 10:34:54 CEST 2022
  * fix validation of paths from command line

  The new procedure makeRelativeTo no longer reduces all occurrences of ".."
  if they occur in a non-existing trailing part of the input path. This means
  we could run into error calls when calling floatSubPath on SubPaths
  resulting from that. So we need to propagate these errors and catch them at
  the call sites.

patch 9e8395ede0848f92cc8dfd517f24a27e8e474f98
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri May 13 12:49:02 CEST 2022
  * remove obsolete Darcs.Util.Path.internalMakeName

patch 91c4ba8b0d14af266e3814bad66565ac9c9f8cc4
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat May  7 09:39:19 CEST 2022
  * simplify tests/issue2275_follows-symlinks.sh

  The test harness nowadays can be run with index enabled and/or disabled so
  we don't need to provide for that in the test script itself.

patch adbbbf6590fe5f6ce82bad41aa5749fa77a9ff3e
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu May  5 15:19:10 CEST 2022
  * extend and generalize test for issue1078

patch f5448554e144a6f76d5a5947f0b00d6452472919
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue May 10 01:56:18 CEST 2022
  * extend, generalize, and rename tests/issue1057.sh

patch b1e5e39e2c37f545137fa428cdc7693223a6d9fc
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri May 13 09:17:46 CEST 2022
  * harness: set environment so we can test symlinks on Windows

  See the URL in the comment for why this is needed.

patch 98dc2fdcf921e1ab73e4212b96fcfbd7dca0a3e3
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri May 13 09:22:18 CEST 2022
  * enable running all symlink tests on Windows

  The only exception is when we use mkfifo in
  tests/issue1645-ignore-symlinks.sh which is very much specific to Posix.

patch caa6b9e51ceb6d79540e8cf456b7dfd8aa9da4c3
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri May 13 09:19:17 CEST 2022
  * tests/lib: disable use of hspwd.hs

  With the recent changes in file path handling this is no longer needed.

patch 19fc6f51a117831fab53fa88d5bc1d18ad11637d
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jun 13 00:09:58 CEST 2022
  * remove no longer used isMaliciousSubPath

patch 7e149343ad6a69119dab08c23f613d7c4e1f7f6a
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jun 13 00:18:47 CEST 2022
  * fix haddocks of D.R.Util.Path.makeRelativeTo

Attachments

• patch-preview.txt
• remove-darcs_util_compat_canonfilename.dpatch

Reviewing incrementally.

>  * remove Darcs.Util.Compat.canonFilename
>  * add two clarifying comments
>  * require unix-compat >= 0.6 for improved symlink support

OK

>   * fix all symlink problems, including Windows

> {-# NOINLINE makeRelativeTo #-}
> makeRelativeTo :: AbsolutePath -> AbsolutePath -> IO (Maybe SubPath)

What's with the NOINLINE?

Could you also explain a bit more what's going on with the changes to
`NativeFilePath.` variants? Is it based on an assumption that the
relevant paths will always be coming from the local filesystem rather
than the repo?

>   * Darcs.Util.Path: add missing HasCallStack constraints
>   * make errors in Darcs.Util.Path.decodeWhite explicit

The last message should have said

>   * Darcs.Util.Path: add missing HasCallStack constraints
>   * make errors in Darcs.Util.Path.decodeWhite explicit

OK

The NOINLINE pragma is a remainder from debugging (to avoid recompilation 
of most downstream modules) that I failed to clean up. It could be 
removed but keeping it shouldn't do any harm.

Regarding NativeFilePath: this is just a refactor I failed to mention in 
the patch description: before the patch we imported some functions 
unqualified from System.FilePath = NativeFilePath and some from 
FilePath.Posix. I found this confusing and therefore changed the imports 
of both Native and Posix FilePath to qualified only.

Uses of NativeFilePath now clearly stand out and we may want to review 
which of them are actually needed. For instance, in makeRelativeTo we 
could use the versions from FilePath.Posix.

Basically, the only place we need NativeFilePath is when dealing 
directly with input from the user (command line, defaults, 
environment, configuration files). But command and option arguments 
that denote file paths are (or should be) converted to AbsolutePath 
before doing anything else with them; and this should include 
conversion to Posix style. What this means is that, in principle, we 
should not need NativeFilePath at all, except perhaps where we do 
the native -> posix conversion.

I say "should" because I haven't done any systematic review, but 
overall this is the rule and most commands follow that. If you 
happen to stumble over code that directly uses an argument or 
environment String as a FilePath then please file a bug report (or 
fix it)!

1 patch for repository darcs-unstable@darcs.net:/opt/darcs/screened:

patch 8cfb0106832238508567066a68251647e3edc1f0
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sat Jul  1 21:16:27 BST 2023
  * Drop accidental NOINLINE

  It was unintentionally included in
    patch c30a57ae84c02fe0fd86093d25945ce1f8ef0ac2
    * fix all symlink problems, including Windows

Attachments

• drop-accidental-noinline.dpatch
• patch-preview.txt

>   * fix all symlink problems, including Windows

OK - I haven't managed to understand this deeply enough to be
intrinsically convinced by it, but hopefully the tests provide
good coverage!

Rest are all fine. Note I'm not running the tests on Windows myself
nowadays, though maybe I should start again. So hopefully CI is
checking properly :-)

Occasionally running the tests manually on Windows might indeed be a 
good idea. The CI runs tests but not the full matrix (that's awfully 
slow on Windows); also, CI test runs are limited to the oldest and 
newest supported compiler versions.