search

wtsi-npg / npg_conda

NPG Conda recipes and tools
1 stars 15 forks source link

ABI lib deps #245

Closed dkj closed 3 years ago

dkj commented 3 years ago

@kjsanger @mksanger Possible way of dealing with ABI dependencies explicitly - just zlib here - we'd need to adapt other lib recipes and ensure tools used the (non-version name) lib dependency in their host dep. Note, the way it's done you might be able to skip the dep in the host dependency to get static lib compilation for the tool - but that would be a (bonus flexibility) side-effect ...

dkj commented 3 years ago

Note build strings are I think wrong:

INFO :: The inputs making up the hashes for the built packages are as follows:
{                                                                                                                              
  "libz-1.2.11-3": {                                             
    "recipe": {}                                                                                                                                                                                                   
  },                                                            
  "libz-dev-1.2.11-3": {                                                                           
    "recipe": {}                                                                                                                                                                                                   
  },                                                            
  "libz1-1.2.11-3": {                                                                                                                                                                                              
    "recipe": {}                                                  
  },                                                                                                                                                                                                               
  "zlib-1.2.11-h9bf148f_3": {                                              
    "recipe": {                                
      "c_compiler": "gcc", 
      "target_platform": "linux-64"                                               
    }                                                                    
  }                                                                       
}

zlib gets build string h9bf148f_3 but libz libz-dev and libz1 get build strings of 3 - seems wrong...

dkj commented 3 years ago

Should we have libgcc-ng dependency? If so on which output packages?

kjsanger commented 3 years ago

Skipping over the implementation details, could you help me understand what this provides over declaring a package requires a version (range) for "reasons" i.e. any reason, including ABI changes?

A recent case was avrocpp which broke compatibilty between 1.8 and 1.9 (API related). That needed a recipe update to declare this, but would not be supported by this method.

It seems the simple method subsumes this more complex one (complex in both research to confirm it's an ABI change and in recipe implementation).

dkj commented 3 years ago

Skipping over the implementation details, could you help me understand what this provides over declaring a package requires a version (range) for "reasons" i.e. any reason, including ABI changes?

start with: libA (ABI1) libB (ABI1) links libA toolA links both libA and libB

then: libB (say a security) update, maintains its ABI1 but also bumps to use new version of libA which has new ABI2 toolA then needs both libA ABI1 (directly linked) and libA ABI2 (linked via libB) (but we don't want to bump libA version for toolA's direct use - may not be supported by toolA upstream yet, or for reproducibility...)

Version ranges cannot cope I think - we need two libA installed. Also, it's difficult to predict what version ranges (which may indicate API compatibility) of libs/tools will maintain ABI compatibility.

Real-life example where @mksanger https://github.com/wtsi-npg/npg_conda/pull/236#issuecomment-830255341 hits same problem @FredDodd6 had with #170

kjsanger commented 3 years ago

Skipping over the implementation details, could you help me understand what this provides over declaring a package requires a version (range) for "reasons" i.e. any reason, including ABI changes?

start with: libA (ABI1) libB (ABI1) links libA toolA links both libA and libB

then: libB (say a security) update, maintains its ABI1 but also bumps to use new version of libA which has new ABI2 toolA then needs both libA ABI1 (directly linked) and libA ABI2 (linked via libB) (but we don't want to bump libA version for toolA's direct use - may not be supported by toolA upstream yet, or for reproducibility...)

Do we have an example where we're prevented from bumping a version used by a tool? Is the example below like that?

Version ranges cannot cope I think - we need two libA installed. Also, it's difficult to predict what version ranges (which may indicate API compatibility) of libs/tools will maintain ABI compatibility.

We already can't predict the effect of version changes for any versioning scheme, even semver, because of differences in version semantcs between packages by different authors, or because of bugs in semver increments, both for ABI and API.

Real-life example where @mksanger #236 (comment) hits same problem @FredDodd6 had with #170

dkj commented 3 years ago

Do we have an example where we're prevented from bumping a version used by a tool? Is the example below like that?

I don't know if biobambam is such a case - we'd have to put a few varied runs through analysis and compare to know for sure. I also don't know where our current packages are missing relevant dependency info to judge how widespread an issue it is - the example given wasn't obvious to spot.

We're protected at the moment by our constrained deployment procedures - but I'm keen we target the (dependency) integrity of our Conda channels (less reliance on our constrained deployment procedures). I think we could rebuild the Biobambam packages after merging #236 - what else do we need to recompile? We'd move further away from the ability to do reproducible deployments, right?

If we could get version dependencies to reflect the ABI dep situation, it would also require us to do more global package rebuilds. I think that is how BioConda/Anaconda deal with this problem: they have sets of build variants (where they build key library versions and trigger many package rebuilds). Would we need to start maintaining such a global file with libraries with ABIs that could change? We would definitely need to ensure the library variants used are encoded in the hash used in the build string, then build all dependent packages on changing one of them. And unless the versions are actually in the dependencies updating a Conda env would still be vulnerable to library problems like in the example - you have to see the tool break and then try upgrading it to get the recompiled version. That is unless we start patching dependencies in Conda indexes - suspect that is another method that Anaconda/Bioconda have to deal with dependency limitations which only become apparent long after the recipe has been used to create packages. Feels hard to understand and like more work...

kjsanger commented 3 years ago

Do we have an example where we're prevented from bumping a version used by a tool? Is the example below like that?

I don't know if biobambam is such a case - we'd have to put a few varied runs through analysis and compare to know for sure. I also don't know where our current packages are missing relevant dependency info to judge how widespread an issue it is - the example given wasn't obvious to spot.

Re. spotting the errors - we can't rely on humans doing it. It takes time and machines do it better.

The effect of changing the co-installed version of a dependency e.g. libfoo from 1.2.0 to 1.3.0 when an application says it needs libfoo (whether or not it specifies a version range), is possibly discoverable by a human reading the respective release notes, or more effectively by an automated test of installing the application into a test environment to see what really happens. We've made some steps in that direction using a generic script for testing any package that will be run on a pull request and also on all packages that are descendants (transitively require the changed package).

We're protected at the moment by our constrained deployment procedures - but I'm keen we target the (dependency) integrity of our Conda channels (less reliance on our constrained deployment procedures). I think we could rebuild the Biobambam packages after merging #236 - what else do we need to recompile? We'd move further away from the ability to do reproducible deployments, right?

We should automate testing to see what needs recompiling - it's too time-consuming and error prone to do it by hand. Even if our dependency declarations are watertight, we still use the Anaconda defaults channel, which is subject to change outside our control.

If we want reproducible deployments, we should export a frozen Conda environment or use containers. IMO it's not practical to try to do that by managing the contents of a Conda channel.

If we could get version dependencies to reflect the ABI dep situation, it would also require us to do more global package rebuilds. I think that is how BioConda/Anaconda deal with this problem: they have sets of build variants (where they build key library versions and trigger many package rebuilds). Would we need to start maintaining such a global file with libraries with ABIs that could change? We would definitely need to ensure the library variants used are encoded in the hash used in the build string, then build all dependent packages on changing one of them. And unless the versions are actually in the dependencies updating a Conda env would still be vulnerable to library problems like in the example - you have to see the tool break and then try upgrading it to get the recompiled version. That is unless we start patching dependencies in Conda indexes - suspect that is another method that Anaconda/Bioconda have to deal with dependency limitations which only become apparent long after the recipe has been used to create packages. Feels hard to understand and like more work...

I think that there should be a test before acceptance of a pull request to demonstrate that it doesn't break other packages (which is work already started). If such a breakage occurs because of incompatibilities (documented, or not), that's the stage to fix things. The updated packages will make it into the devel channel together and won't affect production. I think we should move away from manual curation of lists.

Are there cases not covered by this process (for handling incompatibilities)?

  1. Submit PR
  2. Automated test of recipe build
  3. Automated test of package (once installed)
  4. Automated test of everything (once installed) depending (transitively) on the package
  5. On failure, tighten version declaraions as required, goto 1., on success merge PR
dkj commented 3 years ago

On Tue, 2021-05-04 at 08:21 -0700, Keith James wrote:

Are there cases not covered by this process (for handling incompatibilities)?    1. Submit PR    2. Automated test of recipe build    3. Automated test of package (once installed)    4. Automated test of everything (once installed) depending (transitively) on the package    5. On failure, tighten version declaraions as required, goto 1., on success merge PR

So for the problem Biobambam case with a PR for new versions of Nettle and GNUTLS:

This PR is to thrash out another path - but one which isn't mutually exclusive. It would (if it worked) reduce the number of package rebuilds actually required to maintain a consistent repo.

-- The Wellcome Sanger Institute is operated by Genome Research Limited, a charity registered in England with number 1021457 and a company registered in England with number 2742969, whose registered office is 215 Euston Road, London, NW1 2BE.

dkj commented 3 years ago

Note run_exports is not working as hoped (at 5327365a6673ac0b33ab3180099ee695240794f2): run dependancy libz1 not automatically included when indirect package e.g. libz included in host dep.

dkj commented 3 years ago

Build variants being used to allow for different library versions (and so ABIs) hopefully