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

Allow for better packaging and system integration #13

Closed
manxorist opened this issue Apr 25, 2021 · 3 comments
Closed

Allow for better packaging and system integration #13

manxorist opened this issue Apr 25, 2021 · 3 comments

Comments

@manxorist
Copy link
Contributor

Allow for better packaging and system integration

OpenMPT and libopenmpt (https:://openmpt.org/ and https://lib.openmpt.org/) currently implement PP20, XPK, and MMCMP decompression to allow for loading old module formats. However, in the long term, we probably want to get rid of these implementations in our codebase and use an external library to handle these in OpenMPT (probably actually not in libopenmpt itself because compression formats are better handled by libopenmpt client code in order to reduce dependencies).

In order to be able to use ancient, we would ultimately need it to be packaged in Linux distributions (so that openmpt123 can pick it up as a dependency). Many distributions (in particular Debian) do not allow 3rd party packages (ancient in this case) to be distributed inside of other packages' source trees (libopenmpt in our case), but instead require to use distribution-packages for all dependencies.

In order to allow for friction-free packaging in the context of Linux distributions, there are a couple of things that would need to be (or at least would be great to have) done in ancient:

  1. namespace all internal C++ symbols in namespace ancient

    This avoids any potential symbol conflicts when linking ancient statically.

  2. proper symbol visibility for internal and public APIs

    This avoids any potential symbol conflicts when linking ancient dynamially.

  3. public C++ API header, preferably without inline functions or templates

    ancient should clarify which headers belong to the public API and which do not. I would suggest a minimal subset of Decompressor.hpp and Buffer.hpp here.

  4. public C API (not strictly required for libopenmpt)

    To facilitate using ancient from other programming languages, a C API wrapper would probably be useful.

  5. distribution-friendly build system

    Candidates here are most likely autotools, cmake, or meson. This would be in addition and as an alternative to the current Makefile. I am only familiar with autotools.

  6. stable ABI, in particular also proper soname versioning

    The build system should take care of properly incrementing the library soname when the ABI and/or API changes.

  7. stable API

    The public C and C++ APIs should be designed to be as stable as possible.

As we have already experience with all of these concerns (having done the complete work for libopenmpt here), we would be willing to provide initial implementations and pull requests for all of them.

This issue is intended to gauge whether you would be willing to evolve ancient into that direction. In case you want us to go forward, we can create additional issues and/or pull request to discuss any details.

Cheers,

@manxorist and @sagamusix

@temisu
Copy link
Owner

temisu commented Apr 25, 2021

Hi,

This is very interesting and definitely a good direction to go.

Basically, I think I can address the points 1 to 3. There are indeed some old crud there that could be cleaned out.

point 4 is something that I would not address, unless it is needed by someone - adding dead code just for completeness is not usually a good way to go. Although traditionally C API was seen as mandatory (especially in windows), now I think the trend has reversed. If someone makes PR for it, I would not say no though :)

I haven't had yet any experience with autotools or the like, so point 5 would be place where I would be welcoming any help you can give. (Cmake feels modern, but again, I have not had enough experience on it to make an informed decision)

point 6 and 7 are more of a discussion items (and frankly, something that would be mandator if there would be some sort of buildserver / ci integration) For now the situation is entirely ad-hoc, and can be improved.

In short, yes. I'm willing to take PRs / co-operation / recommendations / discussion to make the project better. And obviously I'm willing to improve the project myself where I have the best context :)

Thanks,
much appreciated

@temisu
Copy link
Owner

temisu commented May 20, 2021

Ok, I think I have cleaned the code around a bit.

I think I have a base for points 1, 2 and 3 but like I said I'm no expert in autoconf so the current Makefile is now a mess (but it should be easily replaceable with a proper build system).

If you wish to contribute this is your chance ;)

@temisu temisu closed this as completed May 20, 2021
@manxorist
Copy link
Contributor Author

Thanks for working on this.

I already started some further cleanups (see pull requests).

Regarding point 3, I would still prefer a proper facade API that does leak even fewer implementation details. I'll open a separate issue for that.

This was referenced May 21, 2021
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

2 participants