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
Metadata Update from @firstyear: - Issue assigned to firstyear
<img alt="0002-Ticket-49139-Update-makefile-and-rpm-for-import.patch" src="/389-ds-base/issue/raw/files/74da7dad40639b4f14948a81056e8ec4a453a02e1d580329efb79e22a01ca2c5-0002-Ticket-49139-Update-makefile-and-rpm-for-import.patch" />
Metadata Update from @firstyear: - Custom field reviewstatus adjusted to review
<img alt="0001-Ticket-49139-Import-libsds-and-nunc-stans-for-bundli.patch" src="/389-ds-base/issue/raw/files/c08204bbdb8d7688546bd78f47a5adaf9295f950eb44abaeca4d303f4b6d8310-0001-Ticket-49139-Import-libsds-and-nunc-stans-for-bundli.patch" />
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:
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.
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.
<img alt="0002-Ticket-49139-Update-makefile-and-rpm-for-import.patch" src="/389-ds-base/issue/raw/files/efde957c5be333ae34940b1305a9ac43dbda0af19168bf64ec24b5d5a2217613-0002-Ticket-49139-Update-makefile-and-rpm-for-import.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.
make -f rpm.mk rpms
make -f rpm.mk sprms
tarball
rpmbuildprep
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.
cp
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.
<img alt="0002-Ticket-49139-Update-makefile-and-rpm-for-import.patch" src="/389-ds-base/issue/raw/files/1f22e7e6be7266ad0608698d5114df912b968fa8e27bc9f71f9ca8f0f3e7440c-0002-Ticket-49139-Update-makefile-and-rpm-for-import.patch" />
Ack
Metadata Update from @mreynolds: - Custom field reviewstatus adjusted to ack (was: review)
Metadata Update from @firstyear: - Custom field type adjusted to defect
<img alt="0001-Ticket-49139-Import-libsds-and-nunc-stans-for-bundli.patch" src="/389-ds-base/issue/raw/files/40cd00cd7a98bf43cab48e5f872e71f785721a2624a004e6add4b9d1c60aa279-0001-Ticket-49139-Import-libsds-and-nunc-stans-for-bundli.patch" />
<img alt="0002-Ticket-49139-Update-makefile-and-rpm-for-import.patch" src="/389-ds-base/issue/raw/files/42cf928349c050a3e994c308cb542c3b346caccb53d992815f4e1df26ad057a7-0002-Ticket-49139-Update-makefile-and-rpm-for-import.patch" />
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)
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/2198
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: fixed)
Login to comment on this ticket.