False positive in ImportPackageFiles when adding .deb files that have different metadata and name but same contents

[ferrreo]

Detailed Description

If you have a bunch of packages that differ only in package name and depends then when you try add them to your repo via repo add and you have forceReplace set then one of them gets booted out even though they do not actually match or conflict.

This is due to ImportPackageFiles inside the force replace block it is doing a list.search but this is not a thorough as package.Equals, if you add check that matches package.Equals minus the filehash in the loop ranging over "conflictingFiles" then the problem goes away.

Ideally the fix would be in list.search or it's params.

Context

This caused some serious confusion when packages went "missing" from our Distro repo.
This is a pretty common usecase when it comes to metapackages, as generally they have no content and just differ by their depends.

Possible Implementation

I fixed it locally with adding a check to package.SoftEquals in the looping over conflictingFiles and only marking as conflicted if the check passes. SoftEquals is just package.Equals minus the fileshash check.

Your Environment

Not really environment specific but I can provided debs or an example control file if needed.

[neolynx]

Hi !

thanks for the analysis :-)

would you mind opening a PR, so we can have a look ?

[neolynx]

is there an easy way to reproduce this ?

[ferrreo]

Apologies, been super busy since I raised this. To replicate try and add two debs that have different names and different deps in the control (common for virtual packages) at the same time (e.g repo add a dir with them both in).

Here are two of our debs we had issues with:

https://ppa.pika-os.com/pool/cockatiel/m/mesa/mesa-stable_24.2.6-101pika1_amd64.deb
https://ppa.pika-os.com/pool/cockatiel/m/mesa/mesa-stable-no-march_24.2.6-101pika1_amd64.deb

I can provide the control file too if needed.

The code I used to "fix" it:

In import.go:

if forceReplace { conflictingPackages := list.Search(Dependency{Pkg: p.Name, Version: p.Version, Relation: VersionEqual, Architecture: p.Architecture}, true) for _, cp := range conflictingPackages { if !cp.SoftEquals(p) { reporter.Warning("%s marked as conflicted but didn't match", p) continue } reporter.Removed("%s removed due to conflict with package being added", cp) list.Remove(cp) } }

package.go

func (p *Package) SoftEquals(p2 *Package) bool { return p.Name == p2.Name && p.Version == p2.Version && p.SourceArchitecture == p2.SourceArchitecture && p.Architecture == p2.Architecture && p.Source == p2.Source && p.IsSource == p2.IsSource }

This was very much a hack to stop packages randomly vanishing from our production repo, I don't have the time to properly root cause at the moment but my gut feeling is the list.Search isn't quite doing what it should be here.

[neolynx]

Indeed, I added the 1st package to a new repo, then adding the 2nd results in:

{
  "FailedFiles": [],
  "Report": {
    "Warnings": [],
    "Added": [
      "mesa-stable-no-march_24.2.6-101pika1_amd64 added"
    ],
    "Removed": [
      "mesa-stable-no-march_24.2.6-101pika1_amd64 removed due to conflict with package being added"
    ]
  }
}

[ferrreo]

That's the issue yeah, neither should get removed because they are different packages with different names and different metadata.

[neolynx]

it only seems to happen if both packages are added at the same time...

with your quick fix I get:

{
  "FailedFiles": [],
  "Report": {
    "Warnings": [
      "mesa-stable_24.2.6-101pika1_amd64 marked as conflicted but didn't match"
    ],
    "Added": [
      "mesa-stable-no-march_24.2.6-101pika1_amd64 added",
      "mesa-stable_24.2.6-101pika1_amd64 added"
    ],
    "Removed": []
  }
}

I agree, the list.search should probably return only real conflicting packages...

[neolynx]

@5hir0kur0 could you maybe have a look at this ?

I think the change in func (l *PackageList) Search(dep Dependency, allMatches bool) from ab18d48 might now return provided packages, which prevents other provided packages from being added to a repo in one go..

[neolynx]

reproducer:

curl -O https://ppa.pika-os.com/pool/cockatiel/m/mesa/mesa-stable_24.2.6-101pika1_amd64.deb -O https://ppa.pika-os.com/pool/cockatiel/m/mesa/mesa-stable-no-march_24.2.6-101pika1_amd64.deb

aptly repo create repo1

aptly repo add -force-replace repo1 mesa-stable-no-march_24.2.6-101pika1_amd64.deb mesa-stable_24.2.6-101pika1_amd64.deb

results in:

Loading packages...
conflictingPackages: []
[+] mesa-stable-no-march_24.2.6-101pika1_amd64 added
conflictingPackages: [mesa-stable-no-march_24.2.6-101pika1_amd64]
[-] mesa-stable-no-march_24.2.6-101pika1_amd64 removed due to conflict with package being added
[+] mesa-stable_24.2.6-101pika1_amd64 added

[5hir0kur0]

Yes, ab18d48 seems to be the commit that introduces this behavior.

The old version in ff8a029 of Search was only considering Provides when the Relation was VersionDontCare:

	if dep.Relation == VersionDontCare {
		for _, p := range l.providesIndex[dep.Pkg] {
			if dep.Architecture == "" || p.MatchesArchitecture(dep.Architecture) {
				searchResults = append(searchResults, p)

				if !allMatches {
					break
				}
			}
		}
	}

If you add a || dep.Relation == VersionEqual and check the version to the old commit, you get the same behavior as on master.

Though I personally think that the current version of Search is more correct and that, if anything, it's a problem with the calling code.

I also went and downloaded an ISO from pika-os.com (where the packages in question seem to be from) and apt also reports that they do in fact conflict with each other:

So, is this a bug at all? If I understand correctly, the parameter forceReplace is supposed to remove conflicting packages and it seems to be doing that correctly.

[ferrreo]

@shi

Yes, ab18d48 seems to be the commit that introduces this behavior.

The old version in ff8a029 of Search was only considering Provides when the Relation was VersionDontCare:

	if dep.Relation == VersionDontCare {
		for _, p := range l.providesIndex[dep.Pkg] {
			if dep.Architecture == "" || p.MatchesArchitecture(dep.Architecture) {
				searchResults = append(searchResults, p)

				if !allMatches {
					break
				}
			}
		}
	}

If you add a || dep.Relation == VersionEqual and check the version to the old commit, you get the same behavior as on master.

Though I personally think that the current version of Search is more correct and that, if anything, it's a problem with the calling code.

I also went and downloaded an ISO from pika-os.com (where the packages in question seem to be from) and apt also reports that they do in fact conflict with each other:

So, is this a bug at all? If I understand correctly, the parameter forceReplace is supposed to remove conflicting packages and it seems to be doing that correctly.

apt is saying they conflict because we have conflicts set in the control file. apt dependency conflicts should have no bearing on if it gets added to the repo or not. It is normal to have packages that have the same provides to conflict with each other. It is how Debian based systems offer choices for specific functionality without allowing the user to have multiple things installed doing the same thing.

I think there is a misunderstanding on how conflicts in apt and apt repositories should work. Client install conflicts are not a concern of the repository, they are to prevent the user installing two packages that do not work together, they are not there to prevent both packages being available in the repo for the user to install.

It is not correct to be considering the provides at all when adding packages to the repo (at least in this type of add where we are manually adding a set if packages and not following deps or anything on an import). It is 100% correct to have multiple packages providing the same virtual package in the same repo and you can see this all over in the Ubuntu and Debian repos. We should be checking the name, version and architecture, if one of these things does not match then it is not a conflict in the context of adding to the repo regardless of if anything else matches.

[neolynx]

The packages are only conflicting on installation time, in the repository they can coexist.

I think in aptly we have both scenarios:

1. search for packages, including meta packages
   list.Search should also return provided packages, i.e. for mirroring, snapshotting

2. search for duplicates / conflicting packages
   list.Search should not return provided/meta packages, otherwise this bug is triggered

@5hir0kur0 what do you think of the approach here: #1386 ?

[5hir0kur0]

@neolynx #1386 looks good to me, thanks! 👍🏼

[neolynx]

@ferrreo fi has been released to master, please try latest CI build ! :-)

[ferrreo]

@ferrreo fi has been released to master, please try latest CI build ! :-)

Working great, thanks a lot for the quick work!