New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Files from conflicting packages might override each other #65

Closed
smira opened this Issue Jun 9, 2014 · 16 comments

Comments

Projects
None yet
4 participants
@smira
Member

smira commented Jun 9, 2014

Reported by Yuriy Poltorak:

aptly correctly would handle files with same name, but different md5 from conflicting packages on all stages before publishing. When publishing, conflicting packages may not be part of the same list, but, when publishing under the same prefix into different distrubutions, two publishes would share common pool. Files with the same name (but different content) would override each other silently.

aptly should check for linking files under the same name with different size (checking for md5 might be too expensive).

@smira smira added this to the v0.7 milestone Jun 9, 2014

@smira smira added the bug label Jun 9, 2014

@nightlyone

This comment has been minimized.

Show comment
Hide comment
@nightlyone

nightlyone Jun 10, 2014

The checksums are part of the package list metadata (Packages).
So it might be cheaper than you think, once they are stored.

see http://cdn.debian.net/debian/dists/wheezy/main/binary-amd64/Packages.gz

Package: 0ad
Version: 0~r11863-2
Installed-Size: 8260
Maintainer: Debian Games Team <pkg-games-devel@lists.alioth.debian.org>
Architecture: amd64
Depends: 0ad-data (>= 0~r11863), 0ad-data (<= 0~r11863-2), gamin | fam, libboost-signals1.49.0 (>= 1.49.0-1), libc6 (>= 2.11), libcurl3-gnutls (>= 7.16.2), libenet1a, libgamin0 | libfam0, libgcc1 (>= 1:4.1.1), libgl1-mesa-glx | libgl1, libjpeg8 (>= 8c), libmozjs185-1.0 (>= 1.8.5-1.0.0+dfsg), libnvtt2, libopenal1, libpng12-0 (>= 1.2.13-4), libsdl1.2debian (>= 1.2.11), libstdc++6 (>= 4.6), libvorbisfile3 (>= 1.1.2), libwxbase2.8-0 (>= 2.8.12.1), libwxgtk2.8-0 (>= 2.8.12.1), libx11-6, libxcursor1 (>> 1.1.2), libxml2 (>= 2.7.4), zlib1g (>= 1:1.2.0)
Pre-Depends: dpkg (>= 1.15.6~)
Description: Real-time strategy game of ancient warfare
Homepage: http://www.wildfiregames.com/0ad/
Description-md5: d943033bedada21853d2ae54a2578a7b
Tag: game::strategy, implemented-in::c++, interface::x11, role::program,
 uitoolkit::sdl, uitoolkit::wxwidgets, use::gameplaying,
 x11::application
Section: games
Priority: optional
Filename: pool/main/0/0ad/0ad_0~r11863-2_amd64.deb
Size: 2260694
MD5sum: cf71a0098c502ec1933dea41610a79eb
SHA1: aa4a1fdc36498f230b9e38ae0116b23be4f6249e
SHA256: e28066103ecc6996e7a0285646cd2eff59288077d7cc0d22ca3489d28d215c0a

The checksums are part of the package list metadata (Packages).
So it might be cheaper than you think, once they are stored.

see http://cdn.debian.net/debian/dists/wheezy/main/binary-amd64/Packages.gz

Package: 0ad
Version: 0~r11863-2
Installed-Size: 8260
Maintainer: Debian Games Team <pkg-games-devel@lists.alioth.debian.org>
Architecture: amd64
Depends: 0ad-data (>= 0~r11863), 0ad-data (<= 0~r11863-2), gamin | fam, libboost-signals1.49.0 (>= 1.49.0-1), libc6 (>= 2.11), libcurl3-gnutls (>= 7.16.2), libenet1a, libgamin0 | libfam0, libgcc1 (>= 1:4.1.1), libgl1-mesa-glx | libgl1, libjpeg8 (>= 8c), libmozjs185-1.0 (>= 1.8.5-1.0.0+dfsg), libnvtt2, libopenal1, libpng12-0 (>= 1.2.13-4), libsdl1.2debian (>= 1.2.11), libstdc++6 (>= 4.6), libvorbisfile3 (>= 1.1.2), libwxbase2.8-0 (>= 2.8.12.1), libwxgtk2.8-0 (>= 2.8.12.1), libx11-6, libxcursor1 (>> 1.1.2), libxml2 (>= 2.7.4), zlib1g (>= 1:1.2.0)
Pre-Depends: dpkg (>= 1.15.6~)
Description: Real-time strategy game of ancient warfare
Homepage: http://www.wildfiregames.com/0ad/
Description-md5: d943033bedada21853d2ae54a2578a7b
Tag: game::strategy, implemented-in::c++, interface::x11, role::program,
 uitoolkit::sdl, uitoolkit::wxwidgets, use::gameplaying,
 x11::application
Section: games
Priority: optional
Filename: pool/main/0/0ad/0ad_0~r11863-2_amd64.deb
Size: 2260694
MD5sum: cf71a0098c502ec1933dea41610a79eb
SHA1: aa4a1fdc36498f230b9e38ae0116b23be4f6249e
SHA256: e28066103ecc6996e7a0285646cd2eff59288077d7cc0d22ca3489d28d215c0a
@smira

This comment has been minimized.

Show comment
Hide comment
@smira

smira Jun 10, 2014

Member

@nightlyone, this is not that simple. Internally aptly knows MD5 of each file and stores them correctly. When published, files from one prefix/component share one pool directory (public/<prefix>/pool/<component>/...). So each file there might come from any of published distributions for the same prefix/component. It is possible to do that, but not that easy to implement. So file size should be ok.

Member

smira commented Jun 10, 2014

@nightlyone, this is not that simple. Internally aptly knows MD5 of each file and stores them correctly. When published, files from one prefix/component share one pool directory (public/<prefix>/pool/<component>/...). So each file there might come from any of published distributions for the same prefix/component. It is possible to do that, but not that easy to implement. So file size should be ok.

@nightlyone

This comment has been minimized.

Show comment
Hide comment
@nightlyone

nightlyone Jun 10, 2014

It depends whether you do more exhaustive compare after noticing different or equal file size.

If you plan to do an exhaustive compare on equal file size, I agree with you that file size is enough.

If you derive from same name + same size that the file contents are the same, I respectfully disagree on that implementation.

It depends whether you do more exhaustive compare after noticing different or equal file size.

If you plan to do an exhaustive compare on equal file size, I agree with you that file size is enough.

If you derive from same name + same size that the file contents are the same, I respectfully disagree on that implementation.

@smira

This comment has been minimized.

Show comment
Hide comment
@smira

smira Jun 10, 2014

Member

I agree that comparing by size only won't be complete.

Do you have real-life examples of packages that are conflicting (same arch/name/version), have different files (different MD5) but same size?

The way to reproduce this bug is not that simple. Yuriy (who reported the bug) had its own packages built which were conflicting. aptly won't allow to have conflicting packages in one list (snapshot, mirror, local repo...). So Yuriy published two local repositories under the same prefix but different distributions, and package files came into conflict in the pool.

More complete solution would be to keep MD5 of files published files in the DB with their locations in the pool compare MD5 of file being published with MD5 of the file already present.

Member

smira commented Jun 10, 2014

I agree that comparing by size only won't be complete.

Do you have real-life examples of packages that are conflicting (same arch/name/version), have different files (different MD5) but same size?

The way to reproduce this bug is not that simple. Yuriy (who reported the bug) had its own packages built which were conflicting. aptly won't allow to have conflicting packages in one list (snapshot, mirror, local repo...). So Yuriy published two local repositories under the same prefix but different distributions, and package files came into conflict in the pool.

More complete solution would be to keep MD5 of files published files in the DB with their locations in the pool compare MD5 of file being published with MD5 of the file already present.

smira added a commit that referenced this issue Jun 11, 2014

When linking, check that inode file matches if linking to same file. #65


Otherwise files from conflicting packages might override each other in the published
pool. This is explicitly POSIX-only.
@smira

This comment has been minimized.

Show comment
Hide comment
@smira

smira Jun 11, 2014

Member

@nightlyone, I've found simple and bulletproof solution: as aptly is hard-linking files from internal pool to published pool, aptly is now checking that if file already exists, it should have same inode number as source one.

Member

smira commented Jun 11, 2014

@nightlyone, I've found simple and bulletproof solution: as aptly is hard-linking files from internal pool to published pool, aptly is now checking that if file already exists, it should have same inode number as source one.

smira added a commit that referenced this issue Jun 11, 2014

@nightlyone

This comment has been minimized.

Show comment
Hide comment
@nightlyone

nightlyone Jun 11, 2014

Sounds great!

And while importing (e.g. aptly mirror update) it can check the above mentioned metadata to avoid replacing the internal pool entry with a bad package.

Sounds great!

And while importing (e.g. aptly mirror update) it can check the above mentioned metadata to avoid replacing the internal pool entry with a bad package.

@smira

This comment has been minimized.

Show comment
Hide comment
@smira

smira Jun 11, 2014

Member

When importing files aptly stores them by MD5 (it uses first two bytes of MD5) and by filename, so there should be very low chance of collision.

Member

smira commented Jun 11, 2014

When importing files aptly stores them by MD5 (it uses first two bytes of MD5) and by filename, so there should be very low chance of collision.

@divanikus

This comment has been minimized.

Show comment
Hide comment
@divanikus

divanikus Jul 31, 2014

I'm using aptly to publish builds from CI. And I'm experiencing problems with republishing packages with same version.
Say, I have compiled version 2.5.1, inserted it in a repo. But suddenly found that it has wrong dependency library or something like that. So, i'm recompiling it again with same version 2.5.1.
I'm trying to re-insert it and it doesn't work:

sudo aptly repo remove $REPO "${PACKAGE}_${VERSION}_amd64" "${PACKAGE}_${VERSION}_source"
sudo aptly publish update $REPO

sudo aptly repo add $REPO build/*.deb build/*.dsc
sudo aptly publish update $REPO

Version 0.6 allows me to add this package, but i was unable to download it on client, apt says different size.
Version 0.7 complains like this:

ERROR: unable to publish: unable to process packages: error linking file to /var/packages/public/pool/main/m/mypackage/mypackage_1.5.2~2.g27440f7_amd64.deb: file already exists and is different

Only way to fix seems to be drop published repo and republish it again. Which must be quite expensive, i think.

I'm using aptly to publish builds from CI. And I'm experiencing problems with republishing packages with same version.
Say, I have compiled version 2.5.1, inserted it in a repo. But suddenly found that it has wrong dependency library or something like that. So, i'm recompiling it again with same version 2.5.1.
I'm trying to re-insert it and it doesn't work:

sudo aptly repo remove $REPO "${PACKAGE}_${VERSION}_amd64" "${PACKAGE}_${VERSION}_source"
sudo aptly publish update $REPO

sudo aptly repo add $REPO build/*.deb build/*.dsc
sudo aptly publish update $REPO

Version 0.6 allows me to add this package, but i was unable to download it on client, apt says different size.
Version 0.7 complains like this:

ERROR: unable to publish: unable to process packages: error linking file to /var/packages/public/pool/main/m/mypackage/mypackage_1.5.2~2.g27440f7_amd64.deb: file already exists and is different

Only way to fix seems to be drop published repo and republish it again. Which must be quite expensive, i think.

@nightlyone

This comment has been minimized.

Show comment
Hide comment
@nightlyone

nightlyone Jul 31, 2014

@divanikus Never push a package with the same version again. Just always increase the version. This is how Debian packaging is intended to work.

@divanikus Never push a package with the same version again. Just always increase the version. This is how Debian packaging is intended to work.

@divanikus

This comment has been minimized.

Show comment
Hide comment
@divanikus

divanikus Jul 31, 2014

Version is bound to git tag of the package. I see no reason I should increase it. I think that problem is with publish update, like if it doesn't remove info about removed packages.

Version is bound to git tag of the package. I see no reason I should increase it. I think that problem is with publish update, like if it doesn't remove info about removed packages.

@divanikus

This comment has been minimized.

Show comment
Hide comment
@divanikus

divanikus Jul 31, 2014

I've dropped published repo and all it's files are still in place. I think you should clean up pool directory too.

I've dropped published repo and all it's files are still in place. I think you should clean up pool directory too.

@netflash

This comment has been minimized.

Show comment
Hide comment
@netflash

netflash Aug 1, 2014

@divanikus try to do

aptly db cleanup

before publish

netflash commented Aug 1, 2014

@divanikus try to do

aptly db cleanup

before publish

@divanikus

This comment has been minimized.

Show comment
Hide comment
@divanikus

divanikus Aug 2, 2014

Doesn't help either. I was able to fix it by removing the conflicting package and also removing and republishing all published repos.

Doesn't help either. I was able to fix it by removing the conflicting package and also removing and republishing all published repos.

@smira

This comment has been minimized.

Show comment
Hide comment
@smira

smira Aug 4, 2014

Member

@divanikus this is certainly a problem in aptly. The problem is that publish update and publish switch commands do atomic update of published repository. With conflicting packages like you've described it isn't possible to do atomic update. So the fix would be do add a flag to force update in non-atomic fashion for the case with conflicts.

Member

smira commented Aug 4, 2014

@divanikus this is certainly a problem in aptly. The problem is that publish update and publish switch commands do atomic update of published repository. With conflicting packages like you've described it isn't possible to do atomic update. So the fix would be do add a flag to force update in non-atomic fashion for the case with conflicts.

@smira

This comment has been minimized.

Show comment
Hide comment
@smira

smira Aug 4, 2014

Member

On removing packages from the published repository pool: files are present in the pool as long as there's an repository that has the same packages published in that pool. Package pool is shared by all published repos with the same prefix/component. So you might need to drop some published repositories to clean up that file. Or (if you're completely sure), you can remove file manually.

Member

smira commented Aug 4, 2014

On removing packages from the published repository pool: files are present in the pool as long as there's an repository that has the same packages published in that pool. Package pool is shared by all published repos with the same prefix/component. So you might need to drop some published repositories to clean up that file. Or (if you're completely sure), you can remove file manually.

@smira

This comment has been minimized.

Show comment
Hide comment
@smira

smira Aug 4, 2014

Member

I've created separate issue: #90

Member

smira commented Aug 4, 2014

I've created separate issue: #90

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment