Threaded Mode | Linear Mode

3298 · 08-26-2018, 02:22 PM

Another fix incoming, but it's a small one that applies just to the build system.

Code:

Avoid clobbering CFLAGS, LDFLAGS, LDLIBS variables inside the main Makefile

If they are already defined as environment variables, they apparently get passed down with all changes to QEMU's configure script

and thereby to QEMU's Makefile, which chokes on it.

(Discovered while trying to package x49gp for the Arch Linux distro with the 'makepkg' utility, which supplies default flags for compilers that way.)

My computer is running Arch Linux (or rather something based on it), so I figured I might attempt to package up x49gp for installation, since that's fairly easy to do on this distro (and the supported way for system-wide installation of programs, because it lets the package manager track all files in the system directories). However, one trait of the packaging tool made the x49gp build process throw a fit: there are a bunch of definitions for standard Makefile variables in a config file for the tool, which it sets as environment variables, so stuff gets compiled with certain options by default (e.g. CFLAGS=-O2).
Today I figured out what's happening: Apparently setting an environment variable of that name causes the matching variable definitions inside the Makefile to affect that environment variable, which subsequently gets passed down to qemu/qemu-git/configure-small, where they get recorded (including the additions from the main Makefile) for use in QEMU's sub-Makefile. And when the sub-Makefile is subsequently used to compile the QEMU part of x49gp, it goes boom.
My workaround is to avoid writing to variables with those standard names, using similarly named ones (simply prefixed with "X49GP_") instead. The standard variables still get used in the recipes in addition to the custom-named ones, to allow picking up whatever options come in via environment variables. That seems to work just fine, my package got built without the manual hacks I had to use for that job till yesterday.

As a small bonus, I'm attaching the PKGBUILD file (which tells makepkg how to build the package) in case there are other Arch users who happen to be interested in x49gp.
A few notes:
- I wrote "GPL" into the license slot. x49gp has not had a proper license yet, but because QEMU is GPL code, and we're including a modified version of QEMU in x49gp (thus making x49gp a derivative work of QEMU), so it's implicitly GPL. We're probably not conforming to GPL by the letter (no explicit notice about it, let alone a copy of the license text - should probably fix this), but at least we've done so in spirit by distributing it as source. Though to be honest, until recently the reliance on parts of the build system for certain semi-regular tasks (fresh config file, SD card) just made that the most convenient option.
- It's currently pointing to https://github.com/chwdt/x49gp as the source location. That's currently supposed to be the canonical source, but the maintainer still hasn't incorporated my patch series, so until he does, this will fail horribly (at the time I write this, the version over there does not even support being installed). For the time being, I suggest changing the line that reads "source=(git+https://github.com/chwdt/x49gp)" to point to your local repository (e.g. "source=(git+file:///home/you/x49gp)") or maybe Claudio's GitHub repository, who has so far been pretty quick at publishing my patches (thanks!).
- If you plan to submit this PKGBUILD to the AUR (that's the Arch User Repository, not the Advanced Users Reference which the abbreviation normally refers to around here), it may be a good idea to wait until 1. the license thing is properly sorted out, 2. the patch above is in a public repository so it can actually compile out of the box (this will probably happen before the license thing), and 3. that repository has a tag on the master branch, so later commits don't get treated as earlier versions just because their commit hash happens to be numerically smaller than the previous one. I've hacked the version retrieval function in the PKGBUILD to not fail in tag-less situations (like what we have now) so it doesn't bail out, but there is a reason 'git describe' treats tag-less branches as errors by default.
- The filename should be PKGBUILD without any extension, it's just named PKGBUILD.txt to convince the forum software to allow uploading it. Same issue as with the diffs, so you know the drill.
- Don't worry about the dummy version number listed inside, it's automatically updated anytime you run makepkg on it. makepkg is even smart enough to check for updates via git without you having to change the version number manually, and when there's something newer than what it finds next to the PKGBUILD, it'll pull and build, complete with a version number update. An AUR submission should have a proper version number in it, though (just use whatever is the current version when submitting, that appears to be common practice among the VCS-based packages).
- It showcases how I intended the INSTALL_PREFIX mechanism to work with packaging tools: Compile ('make all') with INSTALL_PREFIX pointing to where it ultimately gets installed (because INSTALL_PREFIX is compiled into the binary to allow it to find its installed resources), then install ('make install') with it pointing into the fakeroot that will get processed by the packaging tool. That should be enough to work with pretty much any packaging tool out there.

(08-23-2018 07:15 PM)ijabbott Wrote: I lot of projects started getting that when GCC 8 became the default compiler on their systems. ISTR it is due to calling strncpy with a length that is the same length as the destination buffer. Something that many would argue is a reasonable thing to do if you know what you are doing! (The destination buffer may end up not null-terminated.)

Yes, strncpy was involved, so that's definitely the cause. But I don't get how a definition that usually makes new warnings and errors appear can make this one disappear. Anyway, it works now, and we can worry about fixing it if/when it breaks.
Null termination is indeed not an issue in this case, because the warning location was deep inside the Virtual VFAT driver - the code there was filling part of the FAT structures where things work with fixed maximum length (and space padding when names are shorter) instead of null termination.