libframetime
libframetime copied to clipboard
Multiarch build on 64-bit
On multi-arch 64 bit Linux one might have 32 bit applications too, so the lib needs to be compiled for 32 bit also. Generate both .so files and preload them both and the application will pick the right one.
Isn't generating multilib packages a job of the build system of the distro? The build script can build each just by setting CFLAGS, then move to appropriate dirs/rename. I can't think of any package that would do such a dual build natively?
Some other points:
- what happens when getconf is not installed, but you are on a 64-bit system?
- this breaks the build on pure 64-bit systems, those cannot build 32-bit binaries
- it changes the name, which is an incompatible change. We have downstream users at least in PTS, forcing them to change the scripts is not nice. This could be worked around by having libframetime64.so and libframetime.so, ie no name change for 32-bit
- please avoid space changes (the Makefile patch had added spaces to empty lines, changed the tab before rm)
what happens when getconf is not installed, but you are on a 64-bit system?
Sorry, on Debian getconf is part of an essential package libc-bin so that guarantees it's presence. If it's not present I can maybe add a backup method say via uname -m again part of an essential package coreutils, it might be missing also, so what's next? How many missing programs should we account for?
this breaks the build on pure 64-bit systems, those cannot build 32-bit binaries
Not really, yes the 64 bit version gets build and the 32 one yields an error, but it did not matter in the first place since the pure system can't run 32 bit apps anyway.
it changes the name, which is an incompatible change. We have downstream users at least in PTS, forcing them to change the scripts is not nice.
Actually PTS is the reason why I looked at this, see my post there: http://www.phoronix.com/forums/showthread.php?112433-Phoronix-Test-Suite-5-6-M1-Brings-Stress-Run-Libframetime-Parsing&p=466268#post466268 And yes they'll have to patch their scripts and add both versions at preload so that it's guaranteed that the lib is loaded no matter what arch.
Isn't generating multilib packages a job of the build system of the distro?
The lib comes with PTS and since only Arch Linux packages it at the moment, it does not get to be build as a package most of the time, hence it will be build only for the main arch when PTS installs it for its use cases. Either this gets modified here upstream or in maybe PTS needs it, I thought that having this modification here will cover any downstream in the future. But, yes in the future, when it gets into distro repos this arch check will be rather useless.
please avoid space changes (the Makefile patch had added spaces to empty lines, changed the tab before rm)
I'll edit that when/if any of the desired changes are okayed for another pull.
Sorry, on Debian getconf is part of an essential package libc-bin so that guarantees it's presence. If it's not present I can maybe add a backup method say via uname -m again part of an essential package coreutils, it might be missing also, so what's next? How many missing programs should we account for?
I commonly work with smaller distros with busybox and possibly some other libc than glibc. uname is present in busybox, getconf is not - relying on a common busybox applet like uname is okay.
this breaks the build on pure 64-bit systems, those cannot build 32-bit binaries
Not really, yes the 64 bit version gets build and the 32 one yields an error, but it did not matter in the first place since the pure system can't run 32 bit apps anyway.
It is never okay to throw a screenful full of errors. Guess how many bug reports that will produce? Users will be confused.
it changes the name, which is an incompatible change. We have downstream users at least in PTS, forcing them to change the scripts is not nice.
Actually PTS is the reason why I looked at this, see my post there: http://www.phoronix.com/forums/showthread.php?112433-Phoronix-Test-Suite-5-6-M1-Brings-Stress-Run-Libframetime-Parsing&p=466268#post466268 And yes they'll have to patch their scripts and add both versions at preload.
Please keep the 32-bit name as is. Even if PTS will have to change to enable the 64-bit preload, backwards compatibility is important. Consider an user using a released version of PTS, which may pull the master of libframetime. An incompatible change would break all tests using it for that user.
Isn't generating multilib packages a job of the build system of the distro?
The lib comes with PTS and since only Arch Linux packages it at the moment, it does not get to be build as a package, hence it will be build only for the main arch when PTS installs it for its use cases.
True, though ATM PTS does not use it for any 64-bit test. It could in the future, yes. The right place might then be in the PTS build script for libframetime. Perhaps we should ask Michael for comment?
I'm not opposed to doing it here, to be clear. It just needs to be done in a way that doesn't break anything for any existing user.
True, though ATM PTS does not use it for any 64-bit test. It could in the future, yes.
Both Metro 2033 Redux and Metro Last Light Redux games are 64-bit only and the PTS script uses libframetime as the included benchmark mode is limited.
It is never okay to throw a screenful full of errors. Guess how many bug reports that will produce?
So I need a way to detect if on pure 64, say if no 32 bit libs are available and skip 32 bit build.
@licaon-kter , you should correct the Makefile as well. It is spaced, not tab-indented. Raw file shows spaces.
Also, did you test this? $(CC) is picked up to be gcc on Linux systems, you probably want to orient the Makefile with a $(CXX) definition. I build in clean-room pbuilder environments. If you did get this working (aside from the spaces in the Makefile mistake), let us know what packages you have installed to satisfy this. I haven't tested yet, but I made a build script that just allows amd64 or i386 to be specified for the build, and names the package with libframetime32 or libframetime64.
you should correct the Makefile as well. It is spaced, not tab-indented
Shed painting at most, since it might never get merged anyway.
Also, did you test this?
Worked for me(TM) :)
$(CC) is picked up to be gcc on Linux systems, you probably want to orient the Makefile with a $(CXX) definition.
That's in the original too, maybe you should open a new issue then?
If you did get this working (aside from the spaces in the Makefile mistake), let us know what packages you have installed to satisfy this.
Not sure, I guess you've figured it already :)
I haven't tested yet, but I made a build script that just allows amd64 or i386 to be specified for the build, and names the package with libframetime32 or libframetime64.
This PR yields that too.
Looking at it now, I guess it can be rewritten (since you will/maybe want the 32-bit always):
ifeq ($(ARCH),64)
$(CC) -o $(name64) -shared $(CFLAGS64) $(LDFLAGS) $(src) -ldl -fPIC
strip --strip-unneeded $(name64)
endif
$(CC) -o $(name32) -shared $(CFLAGS32) $(LDFLAGS) $(src) -ldl -fPIC
strip --strip-unneeded $(name32)