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

Build system issues #47

Open
matthijskooijman opened this issue Nov 23, 2021 · 5 comments
Open

Build system issues #47

matthijskooijman opened this issue Nov 23, 2021 · 5 comments

Comments

@matthijskooijman
Copy link
Contributor

matthijskooijman commented Nov 23, 2021

While building the Debian package for the latest release, I came across a few issues:

  1. make generates src/opensfx.sfo, but make clean does not clean it.
  2. The tarball contains an opensfx.obs file, but running make regenerates this file (because opensfx.cat is not present, which is generated and the obs depends on the cat, so the obs is regenerated). Is this intentional? This is somewhat problematic, since the build now (potentially) modifies a file from the tarball, and make clean then removes it, which means that make && make clean leave the build dir in a different state.
  3. make check does no longer work. The Makefile part that handles this seems to use grfid and expect opensfx.grf to exist, which is not the case. Makefile.config does specify MD5_SRC_FILENAME (pointing to the obs file), but that file contains more than just an md5 file, so I'm not exactly sure how this is intended. The old makefile system would ship an opensfx-0.2.3.md5 in the tarball, and then just generate md5sums of the relevant files (only the .cat file I think) and compare those with the shipped file. What is the intention here?
  4. The repo still contains scripts/Makefile.{bundles,def,common}, but these seem unused?
  5. make bundle no longer seems to work to install the bundle into $(DIR_NAME) as before (and as documented). Weirdly enough it just says "make: Nothing to be done for 'bundle'", but I can't actually find a rule for bundle (perhaps some wildcard rule or something). This rule was previously used by the Debian package to install individual files into /usr/share/games/openttd/baseset/opensfx (rather than a tarball), since there is not much point in merging files into a tarball inside a Debian package (though I'm now switching to using make install instead, which also installs separate files).
  6. make install generates a tarball, but does not actually use it (instead, it installs separate files, which is what I'm using now). I think it could just depend on $(DIR_NAME) rather than $(DIR_NAME).tar.
@Wuzzy2
Copy link
Contributor

Wuzzy2 commented Dec 1, 2021

So how serious is this? Does this prevent the package from being accepted into Debian or do you already use workarounds?

If you have workarounds, it would be nice if you share them here. :P

@matthijskooijman
Copy link
Contributor Author

For now, I've ignored problem 1. and 2. This technically violates the Debian policy ("clean (required) This must undo any effects that the build and binary targets may have had", so I'd really like to see this fixed in an upcoming release. I haven't looked at how to actually fix this yet, though.

For problem 3, I have just disabled the md5sum check entirely. But if such a check is still meaningful (e.g. for multiplayer compatibility), I'd like to restore it in a future version. If not, maybe it should be removed from the makefile rather than be broken.

Problem 4 is just packaging hygiene, solution would be to remove these files.

Problem 5 I worked around by using make install instead of make bundle (which is still a little bit of a kludge, because I do not want the version number in the installed directory name).

Problem 6 is just extra work done during install and (I expect) easy to fix with the suggested dependency change.

Hope this helps :-)

@matthijskooijman
Copy link
Contributor Author

Hm, looking more closely, it seems that make install for opensfx actually does install the tarball, rather than individual files. What I wrote is true for OpenMSX, but not OpenSFX. E.g. compare:

$(_V) install -m644 $< $(INSTALL_DIR)
and https://github.com/OpenTTD/OpenMSX/blob/ab7212d88f4ad41cb942f75923825da5a1c1bce7/Makefile#L517

This means that my workaround for problem 5 doesn't actually work (but I can patch the Makefile to be like OpenMSX to make it work anyway), and that point 6 is actually moot, since the dependency is correct for the rule here.

For extra confusion, I see that in OpenGFX, "make install" actually does another thing, so we have:

  1. OpenGFX which installs files separately into $INSTALL_DIR
  2. OpenMSX which installs files into $INSTALL_DIR/openmsx-$VERSION
  3. OpenSFX which installs a tarball $INSTALL_DIR/opensfx-$VERSION.tar

Maybe it would be good to unify these and let them all behave the same? For the Debian package, the GFX version would be the easiest, but maybe other packagers need the other options? I guess it would be fairly easy to implement multiple (e.g. make install, make install_versioned and make install_tar respectively? And maybe option 1 from OpenGFX should also be modified, since now it uses a different interpretation of $INSTALL_DIR (for 1. you should set INSTALL_DIR=/usr/share/games/openttd/baseset/opengfx, but for the others it should be INSTALL_DIR=/usr/share/games/openttd/baseset/, so without the project name in there. OTOH, for OpenSFX the default for Linux is INSTALL_DIR=~/.openttd/baseset/opensfx, so I think with the current makefile and default settings produces an extra "opensfx" in the filename (so it installs ~/openttd/baseset/opensfx/opensfx-1.0.3.tar), which I'm not sure will actually work, while for OpenGFX the interpretation of INSTALL_DIR does match the default value...

@glx22
Copy link
Contributor

glx22 commented Dec 27, 2021

IIRC OpenGFX and OpenSFX can work from tar, but OpenMSX needs to be fully extracted.

@matthijskooijman
Copy link
Contributor Author

IIRC OpenGFX and OpenSFX can work from tar, but OpenMSX needs to be fully extracted.

Ah, good point, OpenTTD can probably read files from tarballs, but (at least with the timidity music driver) music files must be accessible by external programs.

However, for the Debian package, shipping tarballs inside a package is discouraged, since it just creates extra layers and makes it harder to inspect and compare packages, so I'd still need the non-tar version.

matthijskooijman added a commit to matthijskooijman/OpenSFX that referenced this issue Nov 26, 2023
matthijskooijman added a commit to matthijskooijman/OpenSFX that referenced this issue Mar 20, 2024
matthijskooijman added a commit to matthijskooijman/OpenSFX that referenced this issue Mar 20, 2024
LordAro pushed a commit that referenced this issue Mar 20, 2024
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

3 participants