Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bundle nunc-stans and related datastructures #2198

Closed
389-ds-bot opened this issue Sep 13, 2020 · 26 comments
Closed

Bundle nunc-stans and related datastructures #2198

389-ds-bot opened this issue Sep 13, 2020 · 26 comments
Labels
closed: fixed Migration flag - Issue
Milestone

Comments

@389-ds-bot
Copy link

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/49139


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.

@389-ds-bot 389-ds-bot added the closed: fixed Migration flag - Issue label Sep 13, 2020
@389-ds-bot 389-ds-bot added this to the 1.3.6.0 milestone Sep 13, 2020
@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-21 02:48:55

Metadata Update from @Firstyear:

  • Issue set to the milestone: 1.3.6.0

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-21 04:38:16

Metadata Update from @Firstyear:

  • Issue assigned to Firstyear

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-22 07:52:12

0002-Ticket-49139-Update-makefile-and-rpm-for-import.patch

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-22 07:53:41

Metadata Update from @Firstyear:

  • Custom field reviewstatus adjusted to review

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-22 07:58:48

0001-Ticket-49139-Import-libsds-and-nunc-stans-for-bundli.patch

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2017-02-22 08:20:41

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.

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-02-22 17:12:28

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.

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-23 00:11:16

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.

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-23 00:12:01

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.

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2017-02-23 00:34:06

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.

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-23 00:38:15

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!

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2017-02-23 01:26:03

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

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-23 06:26:50

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

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-23 09:41:56

0002-Ticket-49139-Update-makefile-and-rpm-for-import.patch

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2017-02-23 12:51:52

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.

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-24 00:52:12

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.

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-24 01:46:31

0002-Ticket-49139-Update-makefile-and-rpm-for-import.patch

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-02-24 20:52:41

Ack

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2017-02-24 20:52:50

Metadata Update from @mreynolds389:

  • Custom field reviewstatus adjusted to ack (was: review)

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-27 23:10:57

Metadata Update from @Firstyear:

  • Custom field type adjusted to defect

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-27 23:11:48

0001-Ticket-49139-Import-libsds-and-nunc-stans-for-bundli.patch

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-27 23:12:18

0002-Ticket-49139-Update-makefile-and-rpm-for-import.patch

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-27 23:12:35

0002-Ticket-49139-Update-makefile-and-rpm-for-import.patch

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-27 23:13:33

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

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-28 05:18:20

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

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2017-02-28 05:18:51

Metadata Update from @Firstyear:

  • Issue close_status updated to: fixed
  • Issue status updated to: Closed (was: Open)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed: fixed Migration flag - Issue
Projects
None yet
Development

No branches or pull requests

1 participant