How do we know path is a directory?

How come a list of ownerships is a dir?

I seem to be missing some basics about what the algorithm is supposedly doing, and the naming conventions are not helping.

I have changed the code to change the nomenclature to path and to skip regular files completely. It should flag now only "parent-directory-conflicts" which are the ones we care about. There is a small caveat, when we find a conflict with only symlinks we cannot be sure if it is about parent directories. For us to be sure, we would need to look at the link target in the tarball which makes the code much more complex.

Because we have not seen an example of that in the wild, I propose do not overcomplicate the code and if/when we find an example, we can always remove the false positives.

There's nothing about ownership here.

There is but maybe the notion is not the correct one. What I wanted to model is that each directory that is extracted has to be owned by a single package (unless explicitly declared which is outside the scope of the PR). Some other ideas come to find with similar semantic meaning like extractedFrom which goes from path to the package that might extract it. I will change the code to use it instead to see if it is more clear.

The only difference is that for each path we want to split it in groups to see if the content is the same or not which is why we need to record Link and Mode.

I have changed the code to match the example we discussed where there is no notion of ownership, instead we try to group observations. The only difference is that looking for conflicts is not as simple as noting whether len(observations) > 1, we now have to check for inequality.

Where does the "parent" bit come from? Or the directory bit? The logic above seems to handle every path found in the tarball with no checks.

You are right, it comes from the empirical checks but it is better to check that we are in fact correct. The only caveat is that we never know if symlinks point to a file or to a directory and we have to report them anyway, but it is only a conflict if it is a parent directory if not the regular release.validate() logic takes care of it.

EDIT: We will discuss whether we want to make the same assumptions that extract.go does or increase the scope.

The ownership type feels quite poor as a public way to display that information, as the sample data in the tests demonstrate. I would design the desired output first, before any code is written.

Agree, let's discuss offline and change the spec.

... range observations[1:] {

No need to compare it with itself.

This is also a self-contained algorithm, so it may be nicely moved to a helper function, so the code reads:

if hasPathConflict(observations) {
        ...
}

Okay, I will extract it into a function, that makes sense.

Let's compute the observation value ahead of time here, and then delegate the finding to a helper function. Something along the lines of:

index, ok := findObservation(observation, observation)
if !ok {
    observations = append(observations, observation)
}

This is the same note as earlier in this PR: we have very long functions with no attempt to break down algorithms into nice small abstractions.

I followed your suggestion and then realized using slices.IndexFunc will make it concise because the search is encapsulated but without the cost of the extra function and at virtually no extra cost in terms of lines. But please tell me if you think the suggestion is better and I will change it, I have no strong opinion either way.