[pkg] Resolve directory symlinks in fetched targets

[ElectreAAS]

Housekeeping

This PR fixes the tests in #13393
#9873 will not be fixed, but still a significant step towards fixing #13678.

What this PR does

After fetching package sources, add a pass resolving directory symlinks. As they're not problematic at this stage, file symlinks are left as is. Broken symlinks are removed silently to preserve existing behaviour.

Note: both portable_hardlink and portable_symlink work backwards from what I initially understood, see #13791

Done with the help of @Alizter, so thanks :)

[Alizter]

Here is an example that I think causes a loop in the current algorithm:

  /source/
    dir_a/
      link_to_b -> ../dir_b
    dir_b/
      link_to_a -> ../dir_a

Following this along we get an infinitely descending directory.

I think you will need to keep a set of resolved targets and check against it if you want to remember where you have been as to not repeat the resolution.

Another issue that I see is the multiple passes that are currently done, but we can improve this later once we have something that works correctly.

We also might need to add some validation that we don't escape the source directory, I don't think its a good idea to allow symlinks to / for example.

[ElectreAAS]

I just pushed work in progress to adress the latest comment about cycles.
A basic cycle detection mechanism is in place, and works in simple scenarios (basic-cycle.t), but fails in more complex ones (link-to-parent.t) and crashes the sandboxing mechanisms of cram tests. Fun stuff

[ElectreAAS]

CI isn't happy because it finds dir_a/link_to_b/link_to_a before the one I wrote in the test

[ElectreAAS]

Aside from the CI failures due to non-deterministic errors, the main code is ready for review.
Special attention should be given to the different Path.something.t, I had to hit them with a hammer until they looked nice, but I may very well have misunderstood the different invariants at play

[rgrinberg]

I haven't looked closely, but I think your changes might have made that test non-deterministic.

[Alizter]

Here are some problems I've observed:

1. There is a difference between local and tarball sources when it comes to directory symlinks. The tarball sources correctly resolve the contents wheras local sources silently discard them. I think rather than silently discarding them we should either raise a user error if this is something we wish not to support or support it. I would probably consider not supporting it.

2. Broken symlinks appear to be silently ignored. For example something stupid like a symlink to itself. We should probably error to the user in this case saying that we don't accept such symlinks.

[Alizter]

This only canonicalises relative paths and not absolute ones. I think Path.External.canonicalize_abs was the other way you did it.

[ElectreAAS]

For some reason, raw_resolved is always a relative path, which is correctly canonicalized by Path.of_string
I ended up removing Path.External.canonicalize_abs anyway

[Alizter]

Here are some of the issues we currently have:

Test symlink chains: A -> B -> C where intermediate links are also symlinks.

  $ mkdir -p _src/real_dir
  $ echo "content" > _src/real_dir/file.txt
  $ ln -s real_dir _src/link_a
  $ ln -s link_a _src/link_b
  $ ln -s link_b _src/link_c

  $ make_lockdir

Case 1: local source with symlink chain

For local sources, directory symlinks are not copied at all (they are skipped
during the source copy). Only the real directory is present.

  $ make_lockpkg foo <<EOF
  > (version 0.0.1)
  > (source
  >  (fetch
  >   (url file://$PWD/_src)))
  > (build (run cat real_dir/file.txt))
  > EOF

  $ build_pkg foo
  content

Only the real directory is copied - symlink chains are lost entirely:
  $ ls _build/_private/default/.pkg/foo.*/source | sort
  link_a
  link_b
  link_c
  real_dir

Case 2: tarball source with symlink chain

For tarballs, symlinks are preserved during extraction and then resolved.

  $ tar czf _src.tar.gz _src

  $ make_lockpkg bar <<EOF
  > (version 0.0.1)
  > (source
  >  (fetch
  >   (url file://$PWD/_src.tar.gz)))
  > (build (run cat real_dir/file.txt))
  > EOF

  $ build_pkg bar
  content

  $ ls _build/_private/default/.pkg/bar.*/source | sort
  link_a
  link_b
  link_c
  real_dir

BUG: Different behavior between local source and tarball. With local sources,
the symlink chain is completely lost (link_a, link_b, link_c are missing).
With tarballs, all links are resolved to real directories. This inconsistency
means a package behaves differently depending on how it is fetched.

[ElectreAAS]

Update: we've looked further into this and it seems the desired behaviour in Pkg_rules.source_files is rather complicated, almost equating actually supporting symlinks in the engine.
Since the point of this PR was to allow dune pkg to build fetched packages that may contain symlinks, and that we'd added the resolving pass in source_files only for symmetry, we came to the conclusion that it was unnecessary, and have removed it.

I've pushed a new version containing that deletion, along with a few WIP comments, I'm looking into them

[Alizter]

Those OxCaml failures are real and are due to a new version of ppx_expect. It seems let%test_module is now replaced by module%test. I will fix this in another PR.

[Alizter]

Annoyingly OxCaml is using ppx_expect v18, but v18 hasn't been released so we have a situation where the OxCaml version of the build is using a newer library that cannot overlap with the old version we have available. Luckily we can detect that and pass some compatibility flags to make it work.

[ElectreAAS]

As #14382 looks like it's not going to get merged very quickly, I've rebased to exclude it, and inlined the changes into a local is_descendant function. This should separate our concerns for the time being