Lib versioning

[wentasah]

Hi @darrylb123,

I've created a draft pull request from your lib-versioning branch to make it easier to discuss about the code. Treat this as an answer to your email :-)

I'll add comments to the code in a minute.

[darrylb123]

I ended up avoiding make install with the Fedora package. I just copied the files into place and ran ldconfig.

Using ldconfig only creates a link from soname to the actual library. Fedora recommends that the library be versioned and the devel package have the 'linker' library (.so). For the make, make install though I think we need to create the lot as you have suggested. I think I'll add a devel package to Fedora.

I'll test your suggestion with removing the library references in the python build

On Mon, Sep 19, 2022 at 6:58 PM Michal Sojka @.***> wrote:

@.**** commented on this pull request.

Here are my comments.

In Makefile https://github.com/darrylb123/usbrelay/pull/92#discussion_r974008579:

rm -f gitversion.h

$(MAKE) -C usbrelay_py clean

install: usbrelay libusbrelay.so

install -d $(DESTDIR)$(LIBDIR)

• install -m 0755 libusbrelay.so $(DESTDIR)$(LIBDIR)

• install -m 0755 libusbrelay.so.$(USBLIBVER) $(DESTDIR)$(LIBDIR)

• ( cd $(DESTDIR)$(LIBDIR); ln -s libusbrelay.so.$(USBLIBVER) libusbrelay.so ; ln -s libusbrelay.so.$(USBLIBVER) libusbrelay.so.$(USBMAJOR) )

As you wrote in the email, these symlinks are usually created by ldconfig. I think it's OK to do it this way, but maybe, the following would be simpler: ⬇️ Suggested change

• ( cd $(DESTDIR)$(LIBDIR); ln -s libusbrelay.so.$(USBLIBVER) libusbrelay.so ; ln -s libusbrelay.so.$(USBLIBVER) libusbrelay.so.$(USBMAJOR) )

• $(LDCONFIG) -n $(DESTDIR)$(LIBDIR)

At the top of the makefile, you would add something like:

LDCONFIG = /sbin/ldconfig

This works on Debian. If that works also on Fedora, we can leave it like this. Otherwise, we may add some logic to autodetect ldconfig location.

This also works, if the link already exists. ln -s fails, but if you want to stay with ln, we should change this to ln -sf.

In usbrelay_py/setup.py https://github.com/darrylb123/usbrelay/pull/92#discussion_r974010987:

module1 = Extension(

 'usbrelay_py',

• libraries= ['usbrelay'],

• library_dirs= ['./','/usr/lib','/usr/lib64','/usr/lib/x86_64-linux-gnu','/usr/lib/aarch64-linux-gnu','/usr/lib/arm-linux-gnueabihf','..'],

• libraries= [':libusbrelay.so.' + str(USBMAJOR)],

• library_dirs= ['./','/usr/lib','/usr/lib64','/usr/lib/x86_64-linux-gnu','/usr/lib/aarch64-linux-gnu','/usr/lib/arm-linux-gnueabihf','../'],

I think most of those libraries are not necessary. This is the reason why your build tried to link against wrong library. ⬇️ Suggested change

• library_dirs= ['./','/usr/lib','/usr/lib64','/usr/lib/x86_64-linux-gnu','/usr/lib/aarch64-linux-gnu','/usr/lib/arm-linux-gnueabihf','../'],

• library_dirs= ['../'],

---

In Makefile https://github.com/darrylb123/usbrelay/pull/92#discussion_r974012017:

@@ -24,10 +25,11 @@ all: usbrelay libusbrelay.so

libusbrelay.so: libusbrelay.c libusbrelay.h

• $(CC) -shared -fPIC $(CPPFLAGS) $(CFLAGS) $< $(LDFLAGS) -o $@ $(LDLIBS)

• $(CC) -shared -fPIC @.$(USBMAJOR) $(CPPFLAGS) $(CFLAGS) $< $(LDFLAGS) -o @.$(USBLIBVER) $(LDLIBS)

• ln -s libusbrelay.so.$(USBLIBVER) libusbrelay.so.$(USBMAJOR)

I'd use: $(LDCONFIG) -n . (see below).

— Reply to this email directly, view it on GitHub https://github.com/darrylb123/usbrelay/pull/92#pullrequestreview-1111917819, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTSUVIIPUDTRW3JK7NMKB3V7ATLZANCNFSM6AAAAAAQP4KY2E . You are receiving this because you were mentioned.Message ID: @.***>

[darrylb123]

I've incorparated your suggestions into the lib_versioning branch. I have also been getting the fedora devel package to work. The Fedora devel package has the libusbrelay.so link and libusbrelay.h I use ldconfig to make the soname link and it all seems to work correctly and builds correct fedora packages too.

[darrylb123]

First issue.

Issue 2:

Issue

On Mon, 3 Oct 2022, 7:52 pm Michal Sojka, @.***> wrote:

@.**** commented on this pull request.

I think it looks quite OK, but see a few comments below.

In Makefile https://github.com/darrylb123/usbrelay/pull/92#discussion_r985580798:

rm -f gitversion.h

$(MAKE) -C usbrelay_py clean

install: usbrelay libusbrelay.so install -d $(DESTDIR)$(LIBDIR)

• install -m 0755 libusbrelay.so $(DESTDIR)$(LIBDIR)
• install -d $(DESTDIR)$(PREFIX)/bin
• install -m 0755 usbrelay $(DESTDIR)$(PREFIX)/bin
• install -m 0755 libusbrelay.so.$(USBLIBVER) $(DESTDIR)$(LIBDIR)
• $(LDCONFIG) -n $(DESTDIR)$(LIBDIR)
• ( cd $(DESTDIR)$(LIBDIR) ;ln -sr libusbrelay.so.$(USBLIBVER) libusbrelay.so )

I'm not sure this is a good idea. It would be useful if we want to use old usbrelay binary with a new libusbrelay, but this can cause problems in the future. I'd simply drop this line.

So how do we install the binary? I would have thought that if you wanted to keep the old usbrelay binary to try with the new library, you would just take a copy first.

---

In usbrelay_py/setup.py https://github.com/darrylb123/usbrelay/pull/92#discussion_r985584326:

module1 = Extension( 'usbrelay_py',

• libraries= ['usbrelay'],
• library_dirs= ['./','/usr/lib','/usr/lib64','/usr/lib/x86_64-linux-gnu','/usr/lib/aarch64-linux-gnu','/usr/lib/arm-linux-gnueabihf','..'],
• libraries= [':libusbrelay.so.' + str(USBMAJOR)],

What does the colon mean? I cannot find it documented here https://setuptools.pypa.io/en/latest/userguide/ext_modules.html#extension-api-reference .

The colon forces it to link against a particular library file. I found the reference as part of the gcc docs and it works when building the python module.

---

In rpm/usbrelay.spec https://github.com/darrylb123/usbrelay/pull/92#discussion_r985591237:

+%files devel +%{_includedir}/libusbrelay.h +%{_libdir}/libusbrelay.so

I don't think devel packages should contain any .so files, even if they are just symlinks.

It seems this is the recommended way with Fedora devel packages. It sort of makes sense. The main packages have direct references to the library version but if you were going to write code that linked against the package, you would just use -lusbrelay and would need the .so link.

— Reply to this email directly, view it on GitHub https://github.com/darrylb123/usbrelay/pull/92#pullrequestreview-1128089739, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTSUVLYXXMYNVRJ6SVST6DWBKUHNANCNFSM6AAAAAAQP4KY2E . You are receiving this because you were mentioned.Message ID: @.***>

[darrylb123]

From the ld documentation -l namespec--library=namespec

Add the archive or object file specified by namespec to the list of files to link. This option may be used any number of times. If namespec is of the form :filename, ld will search the library path for a file called filename, otherwise it will search the library path for a file called libnamespec.a.

On Mon, 3 Oct 2022, 7:52 pm Michal Sojka, @.***> wrote:

@.**** commented on this pull request.

I think it looks quite OK, but see a few comments below.

In Makefile https://github.com/darrylb123/usbrelay/pull/92#discussion_r985580798:

rm -f gitversion.h

$(MAKE) -C usbrelay_py clean

install: usbrelay libusbrelay.so install -d $(DESTDIR)$(LIBDIR)

• install -m 0755 libusbrelay.so $(DESTDIR)$(LIBDIR)
• install -d $(DESTDIR)$(PREFIX)/bin
• install -m 0755 usbrelay $(DESTDIR)$(PREFIX)/bin
• install -m 0755 libusbrelay.so.$(USBLIBVER) $(DESTDIR)$(LIBDIR)
• $(LDCONFIG) -n $(DESTDIR)$(LIBDIR)
• ( cd $(DESTDIR)$(LIBDIR) ;ln -sr libusbrelay.so.$(USBLIBVER) libusbrelay.so )

I'm not sure this is a good idea. It would be useful if we want to use old usbrelay binary with a new libusbrelay, but this can cause problems in the future. I'd simply drop this line.

In usbrelay_py/setup.py https://github.com/darrylb123/usbrelay/pull/92#discussion_r985584326:

module1 = Extension( 'usbrelay_py',

• libraries= ['usbrelay'],
• library_dirs= ['./','/usr/lib','/usr/lib64','/usr/lib/x86_64-linux-gnu','/usr/lib/aarch64-linux-gnu','/usr/lib/arm-linux-gnueabihf','..'],
• libraries= [':libusbrelay.so.' + str(USBMAJOR)],

What does the colon mean? I cannot find it documented here https://setuptools.pypa.io/en/latest/userguide/ext_modules.html#extension-api-reference .

In rpm/usbrelay.spec https://github.com/darrylb123/usbrelay/pull/92#discussion_r985591237:

+%files devel +%{_includedir}/libusbrelay.h +%{_libdir}/libusbrelay.so

I don't think devel packages should contain any .so files, even if they are just symlinks.

— Reply to this email directly, view it on GitHub https://github.com/darrylb123/usbrelay/pull/92#pullrequestreview-1128089739, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTSUVLYXXMYNVRJ6SVST6DWBKUHNANCNFSM6AAAAAAQP4KY2E . You are receiving this because you were mentioned.Message ID: @.***>

[wentasah]

I don't think devel packages should contain any .so files, even if they are just symlinks.

It seems this is the recommended way with Fedora devel packages. It sort of makes sense. The main packages have direct references to the library version but if you were going to write code that linked against the package, you would just use -lusbrelay and would need the .so link.

You're right. I looked up Debian docs and this is also their preferred way. The reason is that if you call linker like this:

ld -lusbrelay ...

it will find the correct shared library version.

I'm not sure this is a good idea. It would be useful if we want to use old usbrelay binary with a new libusbrelay, but this can cause problems in the future. I'd simply drop this line.

So how do we install the binary? I would have thought that if you wanted to keep the old usbrelay binary to try with the new library, you would just take a copy first.

I was wrong. The symlink is useful for development.

module1 = Extension( 'usbrelay_py',

• libraries= ['usbrelay'],
• library_dirs= ['./','/usr/lib','/usr/lib64','/usr/lib/x86_64-linux-gnu','/usr/lib/aarch64-linux-gnu','/usr/lib/arm-linux-gnueabihf','..'],
• libraries= [':libusbrelay.so.' + str(USBMAJOR)],

What does the colon mean? I cannot find it documented here https://setuptools.pypa.io/en/latest/userguide/ext_modules.html#extension-api-reference .

The colon forces it to link against a particular library file. I found the reference as part of the gcc docs and it works when building the python module.

I see, but this would work only if GNU (or similar) linker is used. It may cause problems, if we want to create a Windows version of the python package, as discussed elsewhere.

If you use just

libraries= ['usbrelay']

and create a libusbrelay.so symlink in the root of the repo (in the top-level Makefile), it works too. And I believe it's more portable.

[darrylb123]

On Tue, 4 Oct 2022, 5:47 pm Michal Sojka, @.***> wrote:

I don't think devel packages should contain any .so files, even if they are just symlinks.

It seems this is the recommended way with Fedora devel packages. It sort of makes sense. The main packages have direct references to the library version but if you were going to write code that linked against the package, you would just use -lusbrelay and would need the .so link.

You're right. I looked up Debian docs and this is also their preferred way. The reason is that if you call linker like this:

ld -lusbrelay ...

it will find the correct shared library version.

I'm not sure this is a good idea. It would be useful if we want to use old usbrelay binary with a new libusbrelay, but this can cause problems in the future. I'd simply drop this line.

So how do we install the binary? I would have thought that if you wanted to keep the old usbrelay binary to try with the new library, you would just take a copy first.

I was wrong. The symlink is useful for development.

module1 = Extension( 'usbrelay_py',

• libraries= ['usbrelay'],
• library_dirs= ['./','/usr/lib','/usr/lib64','/usr/lib/x86_64-linux-gnu','/usr/lib/aarch64-linux-gnu','/usr/lib/arm-linux-gnueabihf','..'],
• libraries= [':libusbrelay.so.' + str(USBMAJOR)],

What does the colon mean? I cannot find it documented here < https://setuptools.pypa.io/en/latest/userguide/ext_modules.html#extension-api-reference

.

The colon forces it to link against a particular library file. I found the reference as part of the gcc docs and it works when building the python module.

I see, but this would work only if GNU (or similar) linker is used. It may cause problems, if we want to create a Windows version of the python package, as discussed elsewhere.

If you use just

libraries= ['usbrelay']

and create a libusbrelay.so symlink in the root of the repo (in the top-level Makefile), it works too. And I believe it's more portable.

If you just do that, the python package requires the .so and therefore breaks library versioning. Been there, done that :) I suggest when we get to try to do a windows port we may need a different makefile or some other tricks. I had thought we would still use the same tool chain on windows so it mightn't matter.

— Reply to this email directly, view it on GitHub https://github.com/darrylb123/usbrelay/pull/92#issuecomment-1266538821, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTSUVPVP2OAHAOWYGSXSR3WBPOIZANCNFSM6AAAAAAQP4KY2E . You are receiving this because you were mentioned.Message ID: @.***>

[wentasah]

If you just do that, the python package requires the .so and therefore breaks library versioning. Been there, done that :)

Really? I tried that and it works well. When I run:

cd usbrelay_py
python3 setup.py build -v

I see:

gcc -shared [-L...] build/temp.linux-x86_64-cpython-310/src/libusbrelay_py.o -L../ -lusbrelay -o build/lib.linux-x86_64-cpython-310/usbrelay_py.cpython-310-x86_64-linux-gnu.so

Note that I removed unimportant -L flags for brevity. You can see that it links the usbrelay library with -lusbrelay. Then, when I check, which library is actually needed by the produced Python extension, I see the correct version:

$ objdump -x build/lib.linux-x86_64-cpython-310/usbrelay_py.cpython-310-x86_64-linux-gnu.so | grep NEEDED
  NEEDED               libusbrelay.so.1
  NEEDED               libc.so.6

I suggest when we get to try to do a windows port we may need a different makefile or some other tricks. I had thought we would still use the same tool chain on windows so it mightn't matter.

We may try using the same compiler, but my preliminary research (via Google) suggests that it's often problematic. Even the official documentation (https://docs.python.org/3/extending/windows.html?highlight=msvc#using-dlls-in-practice) says "Windows Python is built in Microsoft Visual C++; using other compilers may or may not work."

[darrylb123]

I'll try again later, but when I did that a while ago, attempting to run the python extension without the .so link in place failed. I checked with ldd not objdump.

I did have the .so link when I compiled it so I'll try without that too.

Darryl

On Tue, 4 Oct 2022, 10:08 pm Michal Sojka, @.***> wrote:

If you just do that, the python package requires the .so and therefore breaks library versioning. Been there, done that :)

Really? I tried that and it works well. When I run:

cd usbrelay_py python3 setup.py build -v

I see:

gcc -shared [-L...] build/temp.linux-x86_64-cpython-310/src/libusbrelay_py.o -L../ -lusbrelay -o build/lib.linux-x86_64-cpython-310/usbrelay_py.cpython-310-x86_64-linux-gnu.so

Note that I removed unimportant -L flags for brevity. You can see that it links the usbrelay library with -lusbrelay. Then, when I check, which library is actually needed by the produced Python extension, I see the correct version:

$ objdump -x build/lib.linux-x86_64-cpython-310/usbrelay_py.cpython-310-x86_64-linux-gnu.so | grep NEEDED NEEDED libusbrelay.so.1 NEEDED libc.so.6

I suggest when we get to try to do a windows port we may need a different makefile or some other tricks. I had thought we would still use the same tool chain on windows so it mightn't matter.

We may try using the same compiler, but my preliminary research (via Google) suggests that it's often problematic. Even the official documentation ( https://docs.python.org/3/extending/windows.html?highlight=msvc#using-dlls-in-practice) says "Windows Python is built in Microsoft Visual C++; using other compilers may or may not work."

— Reply to this email directly, view it on GitHub https://github.com/darrylb123/usbrelay/pull/92#issuecomment-1266890696, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTSUVMXRHRKYHYYSEL5NQLWBQM2ZANCNFSM6AAAAAAQP4KY2E . You are receiving this because you were mentioned.Message ID: @.***>

[darrylb123]

WIthout the so link it will not build gcc -shared -Wl,-z,relro -Wl,--as-needed -Wl,-z,now -Wl,--build-id=sha1 -g -Wl,-z,relro -Wl,--as-needed -Wl,-z,now -Wl,--build-id=sha1 -g build/temp.linux-x86_64-cpython-310/src/libusbrelay_py.o -L../ -L/usr/lib64 -lusbrelay -o build/lib.linux-x86_64-cpython-310/usbrelay_py.cpython-310-x86_64-linux-gnu.so /usr/bin/ld: cannot find -lusbrelay

With the link it works properly I've added making the so link in the makefile and fixed setup.py as you suggested.

I assume that adding the soname field to the library properly is what resolved the problem I had last time I tried.

[darrylb123]

Michal, Mark has successfully built the libversioning branch for Fedora. I will merge it and create a release when you're happy with it. Mark will use the release to submit for review.

[wentasah]

I'm currently traveling without access to hardware and much time for testing, but I think that the current state of this branch should work well.

[darrylb123]

Thanks, I think it is a good thing. I shall merge it soon.

Darryl

On Thu, Oct 13, 2022 at 7:03 PM Michal Sojka @.***> wrote:

I'm currently traveling without access to hardware and much time for testing, but I think that the current state of this branch should work well.

— Reply to this email directly, view it on GitHub https://github.com/darrylb123/usbrelay/pull/92#issuecomment-1277278380, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTSUVMM2VA34VIFCEBDWGLWC7F5ZANCNFSM6AAAAAAQP4KY2E . You are receiving this because you were mentioned.Message ID: @.***>