#49139 Bundle nunc-stans and related datastructures
Closed: fixed 2 years ago Opened 2 years ago by firstyear.

Issue Description

In order to simplify our build, we should absorb the nunc-stans project into Directory Server.

Give that we are the only consumer of this project this is safe. This gives us a nice guarantee about versions, make's backports and ports easier, and also will keep nunc-stans in step with DS better.


Metadata Update from @firstyear:
- Issue set to the milestone: 1.3.6.0

2 years ago

Metadata Update from @firstyear:
- Issue assigned to firstyear

2 years ago

Metadata Update from @firstyear:
- Custom field reviewstatus adjusted to review

2 years ago

Have you considered using git submodules? This would keep history separate, give control over used versions and relieve us from maintenance hell of different versions of this library in different branches.

So in makefile the libsds stuff uses a bunch of tabs(like WAY too many per line). I don't think this was intentional, and it will throw the format off significantly. The rest looks fine, but Viktor has some valid concerns.

TL;DR - I do not want to use git submodules, they are an unnecessary complexity, and do not resolve the problem this is actually trying to solve.

Okay. Git submodules makes this harder in my view.

The whole point of this ticket is that we had issues building the RPM because we require multiple sources, have to do linking hacks and then rebuild to distribute. This is now broken because NS deps on SDS for the queue wrapper.

The entire goal here is to tightly link and couple Nunc Stans to DS, because they are so close. This means integration in the Makefile.am and configure. We also want to use SDS in DS to replace some structures, so that needs to be wrapped in there too.

As a result, you can't use git submodules for such an integration. Git Submodules is also still going to have the rpm build problems. Git submodules adds another complexity and solves no problems for us IMO. For example, how do you propose we keep the makefile and configure in sync in a way that works with git submodules, AND tar balls? (I don't think you can :) )

This solution is actually EASIER for maintenance.

We have never use Nunc Stans in production before, so having it from Day 1 in the source tree means:

  • 1 patch. A patch to NS is a patch to DS, all interfaces are kept in sync. No need for NS patch, release, then DS patch and release, then updates to everything.
  • We are not likely to change too much in the space, so going toward 1.4, we keep the exact same code layout and build so we are ready to backport patches to 1.3 easily.

As a result, I still stand by the approach I am taking here because it is the simplest from a git, rpm build and autotools perspective. I'm all for simplicity in our system, because I think being too fancy with things like git will bite us eventually and will cause other problems.

So in makefile the libsds stuff uses a bunch of tabs(like WAY too many per line). I don't think this was intentional, and it will throw the format off significantly. The rest looks fine, but Viktor has some valid concerns.

That was how I had it in the other repo. I can clean that up now and submit a new Makefile patch if you like.

TL;DR - I do not want to use git submodules, they are an unnecessary complexity, and do not resolve the problem this is actually trying to solve.
Okay. Git submodules makes this harder in my view.
The whole point of this ticket is that we had issues building the RPM because we require multiple sources, have to do linking hacks and then rebuild to distribute. This is now broken because NS deps on SDS for the queue wrapper.
The entire goal here is to tightly link and couple Nunc Stans to DS, because they are so close. This means integration in the Makefile.am and configure. We also want to use SDS in DS to replace some structures, so that needs to be wrapped in there too.
As a result, you can't use git submodules for such an integration. Git Submodules is also still going to have the rpm build problems. Git submodules adds another complexity and solves no problems for us IMO. For example, how do you propose we keep the makefile and configure in sync in a way that works with git submodules, AND tar balls? (I don't think you can :) )

Thanks for the explanation. Original description didn't mention these details and problems that you tried to solve. I'm OK with your approach.

No problem. Sometimes I have to remember to type out all the logic that is hiding in my mind :)

You'll be happy to know that all the newly merged code comes with tests, and I wired them into make check and the rpm build %check step, which is a nice move for testing in the project too!

rpm build fails for me:

/usr/bin/mkdir -p '/builddir/build/BUILDROOT/389-ds-base-1.3.6.1-20170223gitc825468.fc25.x86_64/usr/share/dirsrv/updates'
/usr/bin/install -c ldap/admin/src/scripts/exampleupdate.sh '/builddir/build/BUILDROOT/389-ds-base-1.3.6.1-20170223gitc825468.fc25.x86_64/usr/share/dirsrv/updates'
make[2]: Leaving directory '/builddir/build/BUILD/389-ds-base-1.3.6.1.20170223gitc825468'
make[1]: Leaving directory '/builddir/build/BUILD/389-ds-base-1.3.6.1.20170223gitc825468'
+ cp -r /builddir/build/BUILD/389-ds-base-1.3.6.1/man/man3 /builddir/build/BUILDROOT/389-ds-base-1.3.6.1-20170223gitc825468.fc25.x86_64//usr/share/man/man3
cp: cannot stat '/builddir/build/BUILD/389-ds-base-1.3.6.1/man/man3': No such file or directory
error: Bad exit status from /var/tmp/rpm-tmp.S8gdmL (%install)
    Bad exit status from /var/tmp/rpm-tmp.S8gdmL (%install)

To reproduce

   $ mock -r fedora-25-x86_64 rebuild 389-ds-base-1.3.6.1-20170223gitc825468.fc25.src.rpm

Looks like a missing define in crc32c.c on non x86_64 systems. I'll add a patch.

make -f rpm.mk rpms and make -f rpm.mk sprms are broken because NUNC_STANS_URL doesn't exist anymore and tarball and rpmbuildprep targets are failing because of that. rpm.mk needs a cleanup.

But even after the cleanup rpm build fails because there are no docs generated using doxygen. I see that we have a dep, but it's not executed anywhere -> cp fails to copy generated man pages.

make -f rpm.mk doesn't take a bunch of stuff into account. It hacks stuff in the rpm spec template that is not consistent with with our updates and it's causing issues.

./configure; make rpms; make srpms

I think I would rather remove this broken script than fix it, because it's creating problems and confusion. For a long time it's been ignored (even by me), and it doesn't match the rest of our build process.

Metadata Update from @mreynolds:
- Custom field reviewstatus adjusted to ack (was: review)

2 years ago

Metadata Update from @firstyear:
- Custom field type adjusted to defect

2 years ago

Fixes the issues with i686 builds. s390 will be resolved in #49141

commit a82a2cc
commit 617c61e
To ssh://git@pagure.io/389-ds-base.git
a95889d..f9351cf master -> master

Metadata Update from @firstyear:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

2 years ago

Login to comment on this ticket.