Skip to content
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

Debian packages for 1.5.2 #263

Closed
satta opened this issue Feb 18, 2014 · 19 comments
Closed

Debian packages for 1.5.2 #263

satta opened this issue Feb 18, 2014 · 19 comments
Assignees

Comments

@satta
Copy link
Member

satta commented Feb 18, 2014

The GenomeTools packages should be updated to include the new upstream version.

@satta satta self-assigned this Feb 18, 2014
@gordon
Copy link
Member

gordon commented Feb 18, 2014

Thanks for handling this, having GenomeTools as a Debian package is so cool :-)

@standage
Copy link
Contributor

👍 Daniel Standage likes this.

@satta
Copy link
Member Author

satta commented Feb 19, 2014

This might take some more time. As it turns out some exported symbols have disappeared from the shared library (https://gist.github.com/satta/1ac2d6e38bd577f17d15). They were not declared as static in 1.5.1 and have -- most likely inadvertently -- been exported as public symbols. In 1.5.2 they have been removed, renamed, etc. now leading to a discrepancy involving missing symbols. This is automatically checked by the package building pipeline and leads to an error that prevents building. It is a condition which normally calls for an ABI version bump, which I would like to avoid as it will also change the package name in the Debian archive.

I found out that only two of the missing symbols appeared in public _api.h headers in 1.5.1:

$ for i in `grep MISSING symbols-missing.txt | cut -f 3 -d' ' | cut -d@ -f1`; do grep -R $i ../genometools-1.5.1/src/*; done | grep api
../genometools-1.5.1/src/core/output_file_api.h:void              gt_output_file_register_options(GtOptionParser *op,
../genometools-1.5.1/src/core/seq_iterator_fastq_api.h:GtSeqIterator* gt_seq_iterator_colorspace_fastq_new(

This luckily makes it quite unlikely that anything linking externally to the Gt shared lib will be broken by the 1.5.2 transition unless it uses one of these functions. It has to be noted that the first one was just renamed to gt_output_file_info_register_options in 1.5.2 and this could easily be fixed in the package by adding a pre-packaging patch introducing a wrapper function. I need to find out what happened to the second one.

Another idea would be to fix 1.5.1 first by having libtool only export those symbols which either appear in an API header or whose names end with '_p' (required by the Ruby bindings but not in any header file). I would need to discuss this issue with some knowledgeable library packagers. Sorry for the delay.

@Garonenur
Copy link
Member

the second one was renamed for naming convention to gt_seq_iterator_fastq_new_colorspace

@satta
Copy link
Member Author

satta commented Feb 20, 2014

Oh, I see. Even better, so would it be enough to add another wrapper for this one too?

@Garonenur
Copy link
Member

I think so. And deprecate it. so we can remove it in the next bigger version?

@satta
Copy link
Member Author

satta commented Feb 20, 2014

I think we should leave the symbol in so we do not break the ABI. The wrapper will only be added in a patch within the Debian package, so you won't have to worry about anything. No need to deprecate anything as the old function name has already been removed from the API headers.

@satta
Copy link
Member Author

satta commented Mar 5, 2014

After getting some response from the Debian mailing lists, it seems it is okay to add the wrappers for these two functions, making them available using their old names. I'd say that if a symbol is not available through the public API headers, then users can not count on them being stable after all.
If we do that we need not increase the ABI version number.
I will also change the Makefile to exclude all symbols which are not in the public API headers from being exported in the shared library. Any objections? If not, I will do this on the weekend if I get around to it.

@gordon
Copy link
Member

gordon commented Mar 6, 2014

No objections. I think it is a good idea to prevent such problems in the future.

@Garonenur
Copy link
Member

sounds good to me.

@satta
Copy link
Member Author

satta commented Mar 10, 2014

I have a first version in my 'exports' branch in https://github.com/satta/genometools/tree/exports which limits the exported symbols to functions and static globals declared in the API headers (over 1000 symbols btw!). Some issues remain:

  • The current approach (linker version script) only works on ELF platforms with GNU ld. Presence of GNU ld is currently checked in the Makefile and on non-GNU systems all symbols are exported just as before. The other way would be to use objcopy to strip away unwanted symbols, but it has the same portability limitations. Libtool is IMHO too complicated to use as it only integrates most nicely with autotools.
  • Some symbols have been used in scripting language bindings while they are not in the public API yet. These are the whole GtScriptWrapperStream, GtScriptWrapperVisitor, GtGraphicsCairo, GtCheckBoundariesVisitor, GtTypeChecker and GtDupFeatureStream classes, a lot of the GtStyle class and the gt_gff3_in_stream_get_used_types, gt_gff3_in_stream_enable_strict_mode, gt_gff3_in_stream_set_type_checker and gt_feature_node_foreach_attribute methods, as well as gt_str_get_mem and gt_array_add_ptr. I have added the former ones to the API and the latter two as special cases to the list of exported functions.

So this should get us going for now, the tests seem to pass for me. Please, please carefully review the changes and test this new slimmed-down library, especially linking from external programs. This includes the Java bindings (should best be tested by @mader).

As always, I am happy to receive feedback! This has been quite a piece of work...

@satta
Copy link
Member Author

satta commented Mar 25, 2014

So, any comments? I would really like to wrap up the new release...

@Garonenur
Copy link
Member

no comment ;) if @mader has nothing to say - do it!

@gordon
Copy link
Member

gordon commented Mar 26, 2014

Looks good to me! Would it make sense to release the code that was added in the meantime as version 1.5.3 and submit that as a Debian package? Can we prevent that much work in the future, maybe with an automated test concerning exported symbols?

@satta
Copy link
Member Author

satta commented Mar 26, 2014

Yes, a new release should be OK (there are some bugfixes, esp. #274, that I'd like to get out there too).
Regarding automated tests, we could add a script comparing the output of nm -g with what is in the API headers. But this would only make sense on GNU systems anyway, as only there we currently have control over what symbols get into the library, on all others we will always have more symbols in the library than in the API. Now on GNU systems we are using version scripts to only expose these symbols, so we will always have only the API symbols in the library. So what situation are you suggesting to test for?
In the Debian context the package building helper tools will keep track of library symbols once we have a basic, clean set of symbols in there. It will even tag symbols with the version of the library when they were first introduced. This mechanism made me aware of this problem in the first place...

@gordon
Copy link
Member

gordon commented Mar 26, 2014

I think it might make sense to have a list of exported symbols for the current release. And then check against that upon release time to make sure no symbols have been dropped accidentally and to make it explicit which symbols have been added. What do you think? Is there a way to deprecate symbols in Debian packages or is that the situation which requires a major version bump?

@satta
Copy link
Member Author

satta commented Mar 27, 2014

Debian uses a symbols file (basically a list of symbols annotated with the version number when they were first introduced -- see http://www.debian.org/doc/manuals/maint-guide/advanced.en.html#librarysymbols) to track interface versions across releases. The package building process for a new upstream version will compile the list of symbols present in the new version and compare it to the symbols file of the previous version. If there are new symbols, they are added with the version number of the new version. If previously available symbols are now missing, one can no longer assume that anything linked to that package version will still function, so then the package ABI version number (and hence the package name) would require a bump (e.g. from libgenometools0 to libgenometools1). IMHO deprecation makes sense here only to encourage developers linking to the library to make their software no longer dependent on the deprecated symbol so it can be removed later -- something which I think must happen outside of Debian.
But as long as you can not be sure everyone has adapted their software, I would say the only way to make sure is to keep old symbols around or to bump the ABI version when they are finally removed.

@satta
Copy link
Member Author

satta commented May 16, 2014

Packages are ready and submitted. However, I have enabled unit tests on the latest version of the package building scripts, discovering errors and hence breaking builds on the platforms s390x, armhf and powerpc. This will keep the new version from migrating to the testing archive as these platforms have been supported before. Now there are two choices:

  • disabling tests again, leaving users for these platforms to run into assertions in some tools, or
  • fixing these problems (I could imagine them having to do with platform specific limits, e.g. value ranges, or endianness -- some of the problems happen in places involving bit-fiddling). I have a Debian qemu image (for qemu-system-ppc) with the GNU toolchain, git, valgrind, gdb etc. installed (https://www.dropbox.com/s/7vxrkj6xz5ogqs4/debian_wheezy_powerpc_standard.qcow2) in which I was able to reproduce these problems and that can be used to track down and fix them. So if anyone wants to have a try...
    I will look at it myself but am a bit caught up with lots of other things to do... sorry
first error: gt_ensure(!strcmp(gt_str_get(unescaped), code)) failed: function gt_gff3_escaping_unit_test, file src/extended/gff3_escaping.c, line 197.
This is probably a bug, please report at https://github.com/genometools/genometools/issues.
gff3 escaping module...error
first error: gt_ensure(__MIN(char) == -128) failed: function gt_safearith_unit_test, file src/core/safearith.c, line 185.
This is probably a bug, please report at https://github.com/genometools/genometools/issues.
safearith module...error
first error: gt_ensure(!gt_tokenizer_has_token(t)) failed: function gt_tokenizer_unit_test, file src/core/tokenizer.c, line 146.
This is probably a bug, please report at https://github.com/genometools/genometools/issues.
tokenizer class...error
Assertion failed: (!err || !gt_error_is_set(err)), function gt_translator_next_with_start, file src/core/translator.c, line 184.
This is a bug, please report it at https://github.com/genometools/genometools/issues.

@satta
Copy link
Member Author

satta commented May 20, 2014

Fixed and uploaded. Packages are building now.

@satta satta closed this as completed May 20, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants