Are there any remaining items that either of you see needing to be addressed before this can be committed?
On Wed, Sep 27, 2023 at 4:44 PM JR Aquino <tana...@gmail.com> wrote: > > Also curious why the do-install target had to be manually written out, > > what's wrong with the autoconf installation? If you need to add extra > > files manually, do it in a post-install step. > > Sorted out the install issues. > > Removed the do-install and now have a proper post-install only for the > extra files. > > This should be the last major item that was outstanding > > Thank you for your patience > > On Wed, Sep 27, 2023 at 11:27 AM JR Aquino <tana...@gmail.com> wrote: > >> Thank you both very much for the feedback! I have been very hard at work >> making the requested changes and corrections. >> >> Attached you will find the updated .tgz with all of the requested >> optimizations. >> >> Responses in-line below: >> >> On 2023/09/26 19:27, A Tammy wrote: >> > >> > On 9/26/23 18:52, JR Aquino wrote: >> > > Attached you will find the proposed new addition of the UnrealIRCD >> Port for >> > > OpenBSD >> > > >> > > I have tested everything based upon the OpenBSD Porting Guide >> documentation >> > > and everything appears to be in full working order. >> > > >> > > Attached in the tarball >> > >> > >> > thanks, this will be a nice one to have. even better if we can sneak it >> > in by 7.4 (seems unlikely though). >> > >> > A few comments. >> > >> > Remove DISTNAME, EXTRACT_SUFX, and bunch of other values which are >> > already the default. I'd suggest with trying to start removing things >> > and if a clean build of the port still works. This will give you a good >> > idea of whats redundant and what isn't. >> >> I did the clean up of the unnecessary values you referenced. DISTNAME >> couldn't be removed without breaking the port though and I noticed that all >> other ports seem to still have DISTNAME declared. If there is a new method >> for declaring that, I'd be happy to reference the doc and implement the >> best practice there. >> >> > >> > Your BUILD_DEPENDS on gmake should be change to say USE_GMAKE=Yes >> >> Fixed. >> >> > Your CONFIGURE_ARGS seems extremely verbose and it seems very likely >> > that a lot of it can be trimmed down. >> >> This is now trimmed down. >> There were a number of defaults that didn't require declaration and I've >> minimized the args down as far as they will go. >> The path declarations in configure are required because the upstream >> source defaults to expecting the user to install into a fully chrooted >> /home/unrealircd directory. >> >> > The LIB_DEPENDS on libtool is almost certainly incorrect (haven't built >> > the port yet). Maybe you should set USE_LIBTOOL=gnu and see if that >> > suffices - https://man.openbsd.org/bsd.port.mk.5#USE_LIBTOOL >> >> > it uses libtdl >> >> This was an unnecessary declaration and has been removed, cleaned up, and >> verified with "make lib-depends-check" and "make port-lib-depends-check" >> >> > A lot of the variables that you''ve set, like RUNDIR, CONFDIR don't >> seem >> > correct. CONFDIR being inside /usr/local is definitely incorrect, it >> > needs to be just /etc/unrealircd. >> > >> > ${SYSCONFDIR} >> >> The added SUBST_VARS variables have been re-reviewed and corrected to >> reflect their proper locations for OpenBSD. >> >> > Also curious why the do-install target had to be manually written out, >> > what's wrong with the autoconf installation? If you need to add extra >> > files manually, do it in a post-install step. >> >> The upstream source project has a peculiar conf and make process; they >> have homebrew'd a Config script that is a completely rewritten interface to >> autoconf and I believe it is to blame for why I have to manually have the >> do-install target. If one of you see something that I am missing, I'd be >> happy to make the adjustment, but I can't get the port to "make install" >> without the aforementioned "do-install" target. >> >> > there's a bunch of ${WRKINST}/usr/local which should be ${PREFIX} >> though >> > yes better to use autoconf installation >> >> Thank you for calling these out, I have corrected the references and >> ${PREFIX} is now properly in place for the paths that required /usr/local >> >> > others: >> > >> > why the empty pre-configure? >> >> This is totally an oversight. It was unnecessary and has been removed. >> >> > just running "make configure" compiles libargon2 and shouldn't, >> > it should probably use security/argon2 instead. (also the compile >> > run as part of configure uses -O3 -march=native, which will >> > produce broken packages) >> >> This requires some explanation. The upstream source bundles its >> dependencies libraries in case the system is lacking them. These bundles >> are unnecessary and should not be compiled or configured using the >> incorrect " -O3 -march=native". >> >> The reason that libargon2 is attempting to erroneously compile is due to >> a bug in security/argon2 - please see my prior email to the list with the >> subject: "[Update] security/argon2 pkg-config version fix" >> >> The OpenBSD port security/argon2 was not properly declaring the right >> make flags and wasn't capturing the necessary version information for >> pkg-config to pickup; A simple fix that was already acknowledged and >> committed to correction by the mailing list. >> >> > it uses a bunch of SUBST_VARS for things which don't appear in >> > PLIST/etc >> >> This should now be hugely cleaned up in the Makefile and PLIST. I've >> made proper use of the @sample syntax to reflect the /etc configs and >> adjusted the other SUBST_VARS and PLIST info accordingly. >> >> Please double-check but I believe things are in proper order now based >> upon other ports examples. >> >> > RUN_DEPENDS for libraries which should be LIB_DEPENDS >> >> Fixed and double-checked via "make lib-depends-check" and "make >> port-lib-depends-check" >> This should be in order now. >> >> > Overall, a pretty good start, hope this all helps. >> > >> > there is a fair bit of work still to do, it won't make 7.4 >> >> Whew! There was quite a bit of work. Please let me know how things look >> now, and if there are additional deltas/corrections still needed. >> >> Thanks again for the feedback! >> >> -JR >> >> >> On Wed, Sep 27, 2023 at 9:03 AM Stuart Henderson <s...@spacehopper.org> >> wrote: >> >>> On 2023/09/26 19:27, A Tammy wrote: >>> > >>> > On 9/26/23 18:52, JR Aquino wrote: >>> > > Attached you will find the proposed new addition of the UnrealIRCD >>> Port for >>> > > OpenBSD >>> > > >>> > > I have tested everything based upon the OpenBSD Porting Guide >>> documentation >>> > > and everything appears to be in full working order. >>> > > >>> > > Attached in the tarball >>> > >>> > >>> > thanks, this will be a nice one to have. even better if we can sneak it >>> > in by 7.4 (seems unlikely though). >>> > >>> > A few comments. >>> > >>> > Remove DISTNAME, EXTRACT_SUFX, and bunch of other values which are >>> > already the default. I'd suggest with trying to start removing things >>> > and if a clean build of the port still works. This will give you a good >>> > idea of whats redundant and what isn't. >>> > >>> > Your BUILD_DEPENDS on gmake should be change to say USE_GMAKE=Yes >>> > >>> > Your CONFIGURE_ARGS seems extremely verbose and it seems very likely >>> > that a lot of it can be trimmed down. >>> > >>> > The LIB_DEPENDS on libtool is almost certainly incorrect (haven't built >>> > the port yet). Maybe you should set USE_LIBTOOL=gnu and see if that >>> > suffices - https://man.openbsd.org/bsd.port.mk.5#USE_LIBTOOL >>> >>> it uses libltdl >>> >>> > A lot of the variables that you''ve set, like RUNDIR, CONFDIR don't >>> seem >>> > correct. CONFDIR being inside /usr/local is definitely incorrect, it >>> > needs to be just /etc/unrealircd. >>> >>> ${SYSCONFDIR} >>> >>> > Also curious why the do-install target had to be manually written out, >>> > what's wrong with the autoconf installation? If you need to add extra >>> > files manually, do it in a post-install step. >>> >>> there's a bunch of ${WRKINST}/usr/local which should be ${PREFIX} though >>> yes better to use autoconf installation >>> >>> others: >>> >>> why the empty pre-configure? >>> >>> just running "make configure" compiles libargon2 and shouldn't, >>> it should probably use security/argon2 instead. (also the compile >>> run as part of configure uses -O3 -march=native, which will >>> produce broken packages) >>> >>> it uses a bunch of SUBST_VARS for things which don't appear in >>> PLIST/etc >>> >>> RUN_DEPENDS for libraries which should be LIB_DEPENDS >>> >>> > Overall, a pretty good start, hope this all helps. >>> >>> there is a fair bit of work still to do, it won't make 7.4 >>> >>>
