Closed DrDaveD closed 3 years ago
@djw8605 @timtheisen Can somebody approve letting the CI checks run?
@timtheisen Can you please review if this will interfere with any Debian building you're doing? One thing I'm particularly concerned about is the change of debian/source/format to 1.0. I believe that is only necessary on OBS when the name of the directory containing the debian packaging files is in fact "debian" on github as it is here. That's because OBS uses that same directory name for its builds. If the directory is named something like "packaging/debian" or "deb" I have to tell OBS to specifically download more files but then I don't think it cares as much about that format file. It may also be a possibility that I can override just that file on OBS and make it work, I don't think I tried that.
CI running
I don't think anything I changed could cause that error in the RPM Build. I just changed the version number there and added a change log.
There should be a Debian CI build though, in addition.
@ellert Maybe you could comment on the switch in debian/source/format to 1.0, whether or not that would be a problem for a Debian build.
@ellert Maybe you could comment on the switch in debian/source/format to 1.0, whether or not that would be a problem for a Debian build.
What is in the debian directory in the source tarball is unimportant for the build for Debian, since those files are not used and the whole directory is replaced with the files from https://salsa.debian.org/ellert/scitokens-cpp. The files there are adapted to the latest Debian policy version, and uses compat level 13. If the goal with the Debian packaging files in the source is to support older releases, it should not used a compat level that high. The choice of 9 as in the current debian/compat is reasonable.
Source format 1.0 is rarely used. It is mainly for internal Debian stuff that will not be used outside of Debian. It does not support applying patches, all changes must be done in the original source tree and require a new upstream release.
What is the issue with using format 3.0 in OBS? You need to rename the source from name-ver.tar.gz to name_ver.orig.tar.gz (beware of the hyphens and underscores) before you start the build or dpkg-buildpackage will say that the source tarball does not exist.
This doesn't build on Debian 11, Ubuntu 20.10, or Ubuntu 21.04 because of a compilation problem in jwt-cpp. I think that might be fixed by updating that submodule to the latest version
There is a chain of modules here. scitokens-cpp includes the module jwt-cpp, which in turn includes the module picojson.
The fix has been merged in picojson https://github.com/kazuho/picojson/pull/131. scitokens-cpp then needs to include a version of jwt-cpp that includes a fixed version of picojson.
jwt-cpp last updated its picojson 7 month ago, has the fix. scitoken last updated its jwt-cpp version 9 month ago, does not have the fix.
@ellert Maybe you could comment on the switch in debian/source/format to 1.0, whether or not that would be a problem for a Debian build.
What is in the debian directory in the source tarball is unimportant for the build for Debian, since those files are not used and the whole directory is replaced with the files from https://salsa.debian.org/ellert/scitokens-cpp.
Great, so hopefully we can put whatever we want here in github to make OBS happy.
Source format 1.0 is rarely used. It is mainly for internal Debian stuff that will not be used outside of Debian. It does not support applying patches, all changes must be done in the original source tree and require a new upstream release.
What is the issue with using format 3.0 in OBS? You need to rename the source from name-ver.tar.gz to name_ver.orig.tar.gz (beware of the hyphens and underscores) before you start the build or dpkg-buildpackage will say that the source tarball does not exist.
OBS has its own script that it applies to all Debian builds. I can't recall exactly what the error was if it wasn't 1.0, but I suspect it ends up pre-processing everything and creating all the files that dpkg-buildpackage expects for that old format. If the format file provided is something else the build fails.
scitokens-cpp then needs to include a version of jwt-cpp that includes a fixed version of picojson.
I submitted #60 to update to the latest jwt-cpp version.
@timtheisen Is there a separate Debian build of this library for HTCondor that would be affected by this?
The HTCondor Team builds scitokens-cpp for older Debian and Ubuntu platforms that the HTCondor Team supports. When I made the build files, I was shooting for the lowest common denominator (Debian 9). That is why the build files are at debhelper version 9.
I would prefer to stay with source format 3.0, so that I can easily apply patches if necessary for older versions.
I support upgrading to newer versions of jwt.
Finally summary: please update as appropriate. please keep source format 3.0 if possible. I'll take care of making any appropriate patches for older platforms in the HTCondor build glue.
Good news, I found a simple fix for the format issue. I simply had to change the obsupdate.sh script to put Format 3.0
into the scitokens-cpp.dsc file it generates.
@timtheisen Are you ok with the removal of CHECK_SYMBOL_EXISTS("uuid_generate" "uuid/uuid.h" UUID_SYMBOL)
from FindUUID.cmake? The comment said it was for Mac, but it caused failures on my Debian OBS builds. Do you build scitokens-cpp on Mac?
I just looked at our Mac build and we are still building 0.5.0. In any case, we can have that as a patch for Mac builds. Our Mac expert will take a look at it. However, the Mac build process and easily patch that in if needed. I have no problem with this pull request as is.
I sometimes build on my mac. But, it's not a requirement.
Hi,
I can confirm that this breaks the scitokens-cpp build on Mac. I have to revert this patch in order to get UUID detected properly.
Brian
Is there a way in CMake to conditionally do that only on Mac?
Probably - but what broke in OBS? The test looks reasonable to run on any platform (but probably only will succeed on BSDs).
@bbockelm I am glad you asked, because I didn't remember the details. I tested putting it back in my personal OBS account build and it only caused problems on old versions of Debian (<= 8.0) and Ubuntu (<= 16.04) (click on "failed" or "succeeded" to see the build output logs). That's not the way I remembered it, I remember struggling with it for a long time until I removed this piece, and I don't think I would have done that for only old OS versions. I suppose something else changed in the meanwhile that fixed the underlying cause. Now I don't even see a helpful error message in those failed logs, just that the library files are missing. Both failed and succeeded logs have
Looking for uuid_generate - not found
Found UUID : /usr/lib/x86_64-linux-gnu/libuuid.so
but the failed ones just install fewer files and complain about missing files such as this:
dh_install: libscitokens0 missing files: debian/tmp/usr/lib/*/libSciTokens.so.*
dh_install: libscitokens0 missing files: debian/tmp/usr/bin/scitokens-*
dh_install: libscitokens-dev missing files: debian/tmp/usr/lib/*/libSciTokens.so
So I think it could probably be put back. For comparison, here's the official cvmfs-contrib OBS account build.
This adds support for building for Debian in cvmfs-contrib's Opensuse Build System. It updates to version 0.6.3 at the same time because that affects the Debian package.
This doesn't build on Debian 11, Ubuntu 20.10, or Ubuntu 21.04 because of a compilation problem in jwt-cpp. I think that might be fixed by updating that submodule to the latest version, but as it turns out it's not so important because OBS doesn't need to build scitokens-cpp in Debian 11 or Ubuntu 21.04 because it's already natively available there. It still might be wanted for Ubuntu 20.10.
So really, this is only for support on Debian versions older than 11 and Ubuntu versions older than 21.04.