Unexpected behaviour of 'aptly snapshot pull' #78

Closed
queeno opened this Issue Jul 9, 2014 · 10 comments

Comments

Projects
None yet
2 participants
@queeno
Contributor

queeno commented Jul 9, 2014

Hi @smira

Not sure whether this is a proper bug or not, just thought of flagging this up to you.

Say I've got two packages I wish to pull from a snapshot:

aptly snapshot pull empty jenkins_snap dest_snap 'jenkins (= 1.4.1)' 'jenkins-plugin'

where
jenkins has got no dependency and
jenkins-plugin depends on jenkins

The snapshot jenkins contains all the versions of Jenkins ever released.

aptly snapshot pull will initially pull jenkins 1.4.1, however that will get removed from the PackageList in favour of the latest version of jenkins to satisfy the jenkins-plugin's dependencies.
If you use the -no-remove flag, both the latest version of jenkins and version 1.4.1 will get pulled.

@smira smira added the bug label Jul 9, 2014

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

@smira

This comment has been minimized.

Show comment
Hide comment
@smira

smira Jul 9, 2014

Member

Thanks, it is certainly bug, I need to think how to resolve it in the best possible way.

Member

smira commented Jul 9, 2014

Thanks, it is certainly bug, I need to think how to resolve it in the best possible way.

@queeno

This comment has been minimized.

Show comment
Hide comment
@queeno

queeno Jul 10, 2014

Contributor

I would think we should perhaps check all the dependencies (including the dependencies of dependencies) for coherency before starting pulling them.
Also, in the code, we might want to keep trace of the dependencies specified by the user vs the dependencies of those dependencies, giving the former slightly more importance over the latter.

In the case I have presented above, since jekins-plugin depends on VersionDontCare jenkins and the user wishes to pull a specific version of jenkins, then jenkins 1.4.1 should also satisfy jenkins-plugin -> jenkins and get pulled.

We should throw an error instead, if a user attempts to pull a certain version of a dependency, but this conflicts with another dependency of dependency.

For example, say I want to run:

aptly snapshot pull empty jenkins_snap dest_snap 'jenkins (= 1.4.1)' 'jenkins-plugin'

where
jenkins hasn't got any dependencies and
jenkins-plugin depends on jenkins > 1.5

The dependency of jenkins-plugin conflicts with the dependency specified by the user, hence an error should be thrown.

Anyway, thanks for looking into this - good luck with finding a solution :)

Contributor

queeno commented Jul 10, 2014

I would think we should perhaps check all the dependencies (including the dependencies of dependencies) for coherency before starting pulling them.
Also, in the code, we might want to keep trace of the dependencies specified by the user vs the dependencies of those dependencies, giving the former slightly more importance over the latter.

In the case I have presented above, since jekins-plugin depends on VersionDontCare jenkins and the user wishes to pull a specific version of jenkins, then jenkins 1.4.1 should also satisfy jenkins-plugin -> jenkins and get pulled.

We should throw an error instead, if a user attempts to pull a certain version of a dependency, but this conflicts with another dependency of dependency.

For example, say I want to run:

aptly snapshot pull empty jenkins_snap dest_snap 'jenkins (= 1.4.1)' 'jenkins-plugin'

where
jenkins hasn't got any dependencies and
jenkins-plugin depends on jenkins > 1.5

The dependency of jenkins-plugin conflicts with the dependency specified by the user, hence an error should be thrown.

Anyway, thanks for looking into this - good luck with finding a solution :)

@queeno queeno closed this Jul 10, 2014

@queeno queeno reopened this Jul 10, 2014

@smira

This comment has been minimized.

Show comment
Hide comment
@smira

smira Jul 13, 2014

Member

@simonaquino, could you please post information about Jenkins mirror you're using (is it official one?), so that I could reproduce your case exactly?

Member

smira commented Jul 13, 2014

@simonaquino, could you please post information about Jenkins mirror you're using (is it official one?), so that I could reproduce your case exactly?

@queeno

This comment has been minimized.

Show comment
Hide comment
@queeno

queeno Jul 14, 2014

Contributor

Hi @smira

Unfortunately that Jenkins mirror is internal to my organisation. However, we can reproduce the behaviour using git and tig debian packages, where tig depends on git.

  1. Download the following packages:
    (git 1:1.7.2.5-3) (git 1:2.0.1-1) (tig 1.0-2)

    wget http://ftp.uk.debian.org/debian/pool/main/g/git/git_1.7.2.5-3_amd64.deb
    wget http://ftp.uk.debian.org/debian/pool/main/g/git/git_2.0.1-1_amd64.deb
    wget http://ftp.uk.debian.org/debian/pool/main/t/tig/tig_1.0-2_amd64.deb
    
  2. Create a local repo

    aptly repo create myrepo
    
  3. Add packages to myrepo

    aptly repo add myrepo git_1.7.2.5-3_amd64.deb git_2.0.1-1_amd64.deb tig_1.0-2_amd64.deb
    
  4. Create snapshot from myrepo

    aptly snapshot create mysnap from repo myrepo
    
  5. Create empty snapshot

    aptly snapshot create empty empty
    
  6. Pull packages from snapshot

    aptly snapshot pull -architectures=amd64 -dry-run empty mysnap destsnap 'git' 'tig'
    

    This produces the following output:

    root@elasticsearch-1:~/aptly# aptly snapshot pull -architectures=amd64 -dry-run empty mysnap  destsnap 'tig' 'git'
    Dependencies would be pulled into snapshot:
       [empty]: Created as empty
    from snapshot:
       [mysnap]: Snapshot from local repo [myrepo]
    and result would be saved as new snapshot destsnap.
    Loading packages (3)...
    Building indexes...
    [+] tig_1.0-2_amd64 added
    [+] git_1:2.0.1-1_amd64 added
    [-] git_1:2.0.1-1_amd64 removed
    [+] git_1:2.0.1-1_amd64 added
    ....
    

    The destination snapshot contains git and tig at the latest versions.

  7. Now try to pin git to version 1:1.7.2.5-3:

    root@elasticsearch-1:~/aptly# aptly snapshot pull -architectures=amd64 -dry-run empty mysnap destsnap 'tig' 'git (=1:1.7.2.5-3)'
    Dependencies would be pulled into snapshot:
       [empty]: Created as empty
    from snapshot:
       [mysnap]: Snapshot from local repo [myrepo]
    and result would be saved as new snapshot destsnap.
    Loading packages (3)...
    Building indexes...
    [+] tig_1.0-2_amd64 added
    [+] git_1:1.7.2.5-3_amd64 added
    [-] git_1:1.7.2.5-3_amd64 removed
    [+] git_1:2.0.1-1_amd64 added
    ...
    

    The destination snapshot contains the expected version of tig, but the latest version of git, disregarding the pinned version.

Hope this helps!

Contributor

queeno commented Jul 14, 2014

Hi @smira

Unfortunately that Jenkins mirror is internal to my organisation. However, we can reproduce the behaviour using git and tig debian packages, where tig depends on git.

  1. Download the following packages:
    (git 1:1.7.2.5-3) (git 1:2.0.1-1) (tig 1.0-2)

    wget http://ftp.uk.debian.org/debian/pool/main/g/git/git_1.7.2.5-3_amd64.deb
    wget http://ftp.uk.debian.org/debian/pool/main/g/git/git_2.0.1-1_amd64.deb
    wget http://ftp.uk.debian.org/debian/pool/main/t/tig/tig_1.0-2_amd64.deb
    
  2. Create a local repo

    aptly repo create myrepo
    
  3. Add packages to myrepo

    aptly repo add myrepo git_1.7.2.5-3_amd64.deb git_2.0.1-1_amd64.deb tig_1.0-2_amd64.deb
    
  4. Create snapshot from myrepo

    aptly snapshot create mysnap from repo myrepo
    
  5. Create empty snapshot

    aptly snapshot create empty empty
    
  6. Pull packages from snapshot

    aptly snapshot pull -architectures=amd64 -dry-run empty mysnap destsnap 'git' 'tig'
    

    This produces the following output:

    root@elasticsearch-1:~/aptly# aptly snapshot pull -architectures=amd64 -dry-run empty mysnap  destsnap 'tig' 'git'
    Dependencies would be pulled into snapshot:
       [empty]: Created as empty
    from snapshot:
       [mysnap]: Snapshot from local repo [myrepo]
    and result would be saved as new snapshot destsnap.
    Loading packages (3)...
    Building indexes...
    [+] tig_1.0-2_amd64 added
    [+] git_1:2.0.1-1_amd64 added
    [-] git_1:2.0.1-1_amd64 removed
    [+] git_1:2.0.1-1_amd64 added
    ....
    

    The destination snapshot contains git and tig at the latest versions.

  7. Now try to pin git to version 1:1.7.2.5-3:

    root@elasticsearch-1:~/aptly# aptly snapshot pull -architectures=amd64 -dry-run empty mysnap destsnap 'tig' 'git (=1:1.7.2.5-3)'
    Dependencies would be pulled into snapshot:
       [empty]: Created as empty
    from snapshot:
       [mysnap]: Snapshot from local repo [myrepo]
    and result would be saved as new snapshot destsnap.
    Loading packages (3)...
    Building indexes...
    [+] tig_1.0-2_amd64 added
    [+] git_1:1.7.2.5-3_amd64 added
    [-] git_1:1.7.2.5-3_amd64 removed
    [+] git_1:2.0.1-1_amd64 added
    ...
    

    The destination snapshot contains the expected version of tig, but the latest version of git, disregarding the pinned version.

Hope this helps!

@queeno

This comment has been minimized.

Show comment
Hide comment
@queeno

queeno Jul 14, 2014

Contributor

In cmd/snapshot_pull.go, do you reckon we should build a list of missing dependencies and pull them only after we finish pulling the dependencies indicated by the user?
Would this fix the bug?

Contributor

queeno commented Jul 14, 2014

In cmd/snapshot_pull.go, do you reckon we should build a list of missing dependencies and pull them only after we finish pulling the dependencies indicated by the user?
Would this fix the bug?

@smira

This comment has been minimized.

Show comment
Hide comment
@smira

smira Jul 14, 2014

Member

@simonaquino I'm currently rewriting cmd/snapshot_pull.go using PackageList.Filter which does exactly that: first querying packages and then expanding missing dependencies.

Thanks a lot for the the test case!

Member

smira commented Jul 14, 2014

@simonaquino I'm currently rewriting cmd/snapshot_pull.go using PackageList.Filter which does exactly that: first querying packages and then expanding missing dependencies.

Thanks a lot for the the test case!

@queeno

This comment has been minimized.

Show comment
Hide comment
@queeno

queeno Jul 14, 2014

Contributor

Thanks a lot @smira

Can't wait to see the new code, good luck! 👍

Contributor

queeno commented Jul 14, 2014

Thanks a lot @smira

Can't wait to see the new code, good luck! 👍

@smira

This comment has been minimized.

Show comment
Hide comment
@smira

smira Jul 14, 2014

Member

With new version I get:

$ aptly snapshot pull -architectures=amd64 -dry-run empty mysnap destsnap 'git' 'tig'
Dependencies would be pulled into snapshot:
    [empty]: Created as empty
from snapshot:
    [mysnap]: Snapshot from local repo [myrepo]
and result would be saved as new snapshot destsnap.
Loading packages (3)...
Building indexes...
[+] git_1:2.0.1-1_amd64 added
[+] tig_1.0-2_amd64 added

and second case:

$ aptly snapshot pull -architectures=amd64 -dry-run empty mysnap destsnap 'tig' 'git (=1:1.7.2.5-3)'
Dependencies would be pulled into snapshot:
    [empty]: Created as empty
from snapshot:
    [mysnap]: Snapshot from local repo [myrepo]
and result would be saved as new snapshot destsnap.
Loading packages (3)...
Building indexes...
[+] git_1:1.7.2.5-3_amd64 added
[+] tig_1.0-2_amd64 added

Which looks correct, right?

Member

smira commented Jul 14, 2014

With new version I get:

$ aptly snapshot pull -architectures=amd64 -dry-run empty mysnap destsnap 'git' 'tig'
Dependencies would be pulled into snapshot:
    [empty]: Created as empty
from snapshot:
    [mysnap]: Snapshot from local repo [myrepo]
and result would be saved as new snapshot destsnap.
Loading packages (3)...
Building indexes...
[+] git_1:2.0.1-1_amd64 added
[+] tig_1.0-2_amd64 added

and second case:

$ aptly snapshot pull -architectures=amd64 -dry-run empty mysnap destsnap 'tig' 'git (=1:1.7.2.5-3)'
Dependencies would be pulled into snapshot:
    [empty]: Created as empty
from snapshot:
    [mysnap]: Snapshot from local repo [myrepo]
and result would be saved as new snapshot destsnap.
Loading packages (3)...
Building indexes...
[+] git_1:1.7.2.5-3_amd64 added
[+] tig_1.0-2_amd64 added

Which looks correct, right?

@queeno

This comment has been minimized.

Show comment
Hide comment
@queeno

queeno Jul 14, 2014

Contributor

That look entirely correct to me.
Thanks very much indeed for your help!

I look forward to trying out the code as soon as you push it to master :)

Contributor

queeno commented Jul 14, 2014

That look entirely correct to me.
Thanks very much indeed for your help!

I look forward to trying out the code as soon as you push it to master :)

@smira

This comment has been minimized.

Show comment
Hide comment
@smira

smira Jul 14, 2014

Member

Already in master, thanks for detailed bugreport!

Member

smira commented Jul 14, 2014

Already in master, thanks for detailed bugreport!

@smira smira closed this Jul 14, 2014

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