FreeBSD contains smlnj 110.98 from 2020. However, the latest available version, according to https://smlnj.org/ is 110.99.6.1 from October 2024. According to changelogs, there were multiple bugfixes and minor improvements since 110.98. Having the FreeBSD package updated to the new version would be very nice. Straightforward updating the version in the port's Makefile does not work.
Dear Alexey, if you apply the following patch ... <ftp://wrap7.free.de/pub/patch/smlnj.patch.20241213> MD5 (smlnj.patch.20241213) = 727d1b33faff3c0a08cf18a7103f7b67 ... to your ports tree, you should be able to build 110.99.6.1. This is not (yet) intended to be committed, as it did not receive much testing. If you report your results, that would be great. Sorry for the inconvenience:-( Johannes
Unfortunately, compilation fails at the configure stage: Configuring asdl. checking build system type... x86_64-unknown-freebsd14.2 checking host system type... x86_64-unknown-freebsd14.2 checking for gcc... gcc checking whether the C compiler works... no configure: error: in '/usr/home/alexey/ports-wrk/lang/smlnj/work/smlnj-110.99.6.1/asdl': configure: error: C compiler cannot create executables See 'config.log' for more details FAILURE: Configuration of asdl failed. ./config/install.sh: !!! Installation of libraries and programs failed. *** Error code 1 Examining config.log shows the culprit: configure:3575: gcc -O2 -pipe -Wno-error=incompatible-function-pointer-types -fPIC -fstack-protector-strong -fno-strict-aliasing -fstack-protector-strong conftest.c >&5 cc1: error: '-Wno-error=incompatible-function-pointer-types': no option '-Wincompatible-function-pointer-types'; did you mean '-Wincompatible-pointer-types'? ❯ gcc --version gcc (FreeBSD Ports Collection) 13.3.0 Also, there is a warning from portlint, but you probably know.
> Unfortunately, compilation fails at the configure stage: > > Configuring asdl. > [...] > checking for gcc... gcc > [...] > configure:3575: gcc -O2 -pipe -Wno-error=incompatible-function-pointer-types -fPIC -fstack-protector-strong -fno-strict-aliasing -fstack-protector-strong conftest.c >&5 > cc1: error: '-Wno-error=incompatible-function-pointer-types': no option '-Wincompatible-function-pointer-types'; did you mean '-Wincompatible-pointer-types'? This should not happen when building the package with poudriere, because in poudriere's clean jail no gcc would be hanging around. But fortunately passing down CC='${CC}' from the port's Makefile to SML/NJ's own build procedure seems to help as well: <ftp://wrap7.free.de/pub/patch/smlnj.patch.20241215> MD5 (smlnj.patch.20241215) = 0c5dfe976a2f41a73e9b8171331d3ac1 > Also, there is a warning from portlint, but you probably know. If you have an idea how to avoid ${CHOWN}, please let me know. As far as I can see, the warning is less annoying than having stage-qa fail or trying to make USES=shebangfix work during the build phase of the port. Thanks for your feedback and patience! Johannes
The above patch works with gcc installed, but fails with poudriere. The doc.tgz archive from upstream contains Mac OS X resource files, which are extacted by tar. But tar then returns 1 and that's taken seriously by poudriere. The following patch is the same as the one above, except for files/patch-config_unpack, which now contains a workaround for the doc.tgz problem: <ftp://wrap7.free.de/pub/patch/smlnj.patch.20241215> MD5 (smlnj.patch.20241215) = 3ec38cec1aed275c50baf0cd5d75f317 Sorry for the noise Johannes
With the latest patch, I have built it successfully, thanks! Standard ML of New Jersey [Version 110.99.6.1; 64-bit; October 25, 2024] - 1 + 2; val it = 3 : int - Re portlint: I'll look at CHMOD and let you know if I fix it.
This is kinda sweeping the dirt under the rug but cannot you modify config/unpack so that it does chmod right after untarring instead of calling chmod from Makefile? Sorry if I miss something.
Hiding the chmod from portlint by putting it into config/unpack is a cool idea! But fortunately there's a way to calm portlint without "sweeping the dirt under the rug":-) Instead it USES shebangfix and thereby keeps the affected scripts visible in the Makefile. Credits for this solution go to lang/julia/Makefile. The following patch also changes the way the tar return code is handled in config/unpack by generalizing it to all upstream tarballs. A few years ago not only the doc.tgz was "infected" with Apple resource files. The patch works (for me:) with make package and gcc installed, as well as with poudriere testport: <ftp://wrap7.free.de/pub/patch/smlnj.patch.20241219> MD5 (smlnj.patch.20241219) = 5577ebb045f5878fd62c01534c26905d It will take a few days before I can test it on i386 hardware, so meanwhile I'd be glad to read any further comments from you. Johannes
Created attachment 256155 [details] Update lang/smlnj to 110.99.6.1
The above patch to lang/smlnj from comment #7 ... <ftp://wrap7.free.de/pub/patch/smlnj.patch.20241219> MD5 (smlnj.patch.20241219) = 5577ebb045f5878fd62c01534c26905d ... has now been attached to this PR and does the following: - Update to SML/NJ 110.99.6.1, upstream changelog: <https://smlnj.org/dist/working/110.99.6.1/HISTORY.html> - Unbreak building with clang if a gcc is installed as well. - Handle tar's error exit code when upstream tarballs contain Apple resource files. - Calm portlint by invoking fix-shebang during post-build. If a committer considers this patch, that would be great. Output from poudriere testport for amd64 and i386 is available: <http://mesh-j-3.free.de/poudriere/smlnj/110.99.6.1/> Thanks! Johannes ps Submitting several of the patches in lang/smlnj/files to upstream is still an open task:-(
Thanks for your work. I check the Makefile - it's very huge and complex (with many if-else). I'd suggest simplify. Ideas: * post-build if EVERYTHING is set: you can use target-EVERYTHING-on [1] instead of .if * many .for X in Y: can replace it with ':@varname@string@' variable modifier (see man make), for example MLSRCS=${MLSRCDIRS:@srcdir@${MLROOT}/${srcdir}@} * if you place the COMPILER_TYPE and .include <bsd.port.pre.mk> before post-extract can use OPT_VARIABLE [2] to DISTFILES and others [1]: https://docs.freebsd.org/en/books/porters-handbook/book/#options-targets [2]: https://docs.freebsd.org/en/books/porters-handbook/book/#options-variables
Created attachment 256230 [details] improved Makefile for 110.99.6.1 using suggestions from uzsolt@
> I check the Makefile - it's very huge and complex (with many if-else). Yes, unfortunately. Most of this comes from upstream doing nearly all the work in a single script (config/install.sh), and not in seperate steps compatible to the usual style using make fetch patch configure build install. Should we try to split&convert config/install.sh into a Makefile and have a trivial port's Makefile as a result? Would upstream accept our conversion? (And what sould happen to config/install.bat?-) > I'd suggest simplify. Ideas: Great, thanks! > * post-build if EVERYTHING is set: you can use target-EVERYTHING-on > [1] instead of .if Done: pre-fetch, do-configure, do-build, post-build, pre-install. > * many .for X in Y: can replace it with ':@varname@string@' variable > modifier (see man make), for example MLSRCS=${MLSRCDIRS:@srcdir@$ > {MLROOT}/${srcdir}@} Oops, MLSRCS has become obsolete (during the decades of this port;). But MLSRCEXCLUDES and MLSRCEXCLUDEREGEX now use your hint. > * if you place the COMPILER_TYPE and .include <bsd.port.pre.mk> before > post-extract This is already true in the previous versions of the Makefile. > can use OPT_VARIABLE [2] to DISTFILES and others But then EVERYTHING_DISTFILES would have to appear before `.include <bsd.port.pre.mk>` and that would break the sequential "order of appearance" of the DISTFILES+= lines in the Makefile and hence in (the reader's mind and in) `make -V DISTFILES`. So I'd prefer to keep the .if for DISTFILES. It's used for SHEBANG_FILES anyway, which cannot be prefixed by EVERYTHING_. But I agree on the use of EVERYTHING_ for PLIST_SUB. I've attached the resulting Makefile, which works for me. Let me know if you're happy with it too, and then I'll run poudriere testport on it.
(In reply to Johannes 5 from comment #12) Thanks, I'll check it. About install.sh: IMHO if it's cleaner can use it. For example ArchLinux use it (https://gitlab.archlinux.org/archlinux/packaging/packages/smlnj/-/blob/main/PKGBUILD?ref_type=heads). Would be nice separate build and install phase in install.sh too. Would you try it?
> Thanks, I'll check it. After posting it yesterday I noticed a warning from stage-qa on i386: ====> Running Q/A tests (stage-qa) Warning: 'smlnj/bin/.run/run.x86-freebsd.so' is not stripped consider trying INSTALL_TARGET=install-strip or using ${STRIP_CMD} The underlying problem with the runtime library ...freebsd.so is discussed in PR 247421, but it only occurs on amd64, not on i386. The solution for i386 was already in the Makefile - but it had to be activated: % diff Makefile Makefile.new 43c43 < .if ${COMPILER_TYPE} == "clang" --- > .if ${COMPILER_TYPE} == "clang" && ${ARCH} == "amd64" The complete patch set including the fixed Makefile is here: <ftp://wrap7.free.de/pub/patch/smlnj.patch.20241228> MD5 (smlnj.patch.20241228) = bab15df3d0419ec07278196c60af1c51 I'll upload poudriere testport outputs for it tonight. > About install.sh: IMHO if it's cleaner can use it. It's definitely simpler to use config/install.sh "as is":-) > For example ArchLinux use it > (https://gitlab.archlinux.org/archlinux/packaging/packages/smlnj/-/blob/main/PKGBUILD?ref_type=heads). As far as I can see that's essentially boilerplate wrapped around config/install.sh. No patches, no OPTIONS, no staging. Is that what you suggest for the lang/smlnj port? But even if we would drop the OPTIONS (I'd prefer not to;), we would still need patches to make it work on FreeBSD (unless they are all upstreamed). And we do need staging (cd .../lang/smlnj ; fgrep -r STAGEDIR). If you have an idea how to achieve that using config/install.sh in a significantly simpler way than it is done currently, then I'll be glad to learn it. > Would be nice separate build and install phase in install.sh too. > Would you try it? I'm certainly interested, and I wish that one day I'll have the otium to try. But updating lang/smlnj (after 4 years of inactivity:( seems to be more urgent. And after that upstreaming patches (as demanded by swills@ in PR 248431) would be the next step. This is a lot of work (at least for me), which is the reason why it didn't happen yet:-(
(In reply to Johannes 5 from comment #14) About install.sh: if you want options should forget install.sh - as I see it hasn't any options. IMHO the option-specific patching would be harder to maintain. Now I check your patch.
(In reply to Zsolt Udvari from comment #15) .if defined(MLTARGETS) - as I see MLTARGETS is defined everytime (at least it contains heap2asm). So is the .if needed? And what if MLTARGETS isn't defined? The .tmp.sed file would be an empty file (empty for loop), the sed will have an empty command list, so nothing changes. Maybe this target can be simpler: append "request (every MLTARGETS)" to targets.default. Or the order of requests is important? Or more simpler (if the order is irrelevant): two files (mltargets-default and mltargets-everything) in FILESDIR and append the option-specified content to targets.default. Or create two extra patch files [1]. do-configure-RECOMPILE-on: creating a file with ECHO_CMDs. What about create a file (targets-recompile.customized) in FILESDIR and copy it to config/targets.customized? And the comment can be in this file. So this target would be only one line. ML*PATCHES*_CMD: the target is create a file list. You can use != assignment in Makefile to set a value to a command's stdout. In this case you don't need backticks in do-build. Does the install.sh use MLRUNTIMEPATCHES (and the others) environment variable? I grepped everything but I couldn't find MLRUNTIMEPATCHES. Uhhh, sorry for more work. If you don't have time can commit the patch from FTP and later (in next year :) ) simplify. [1]: https://docs.freebsd.org/en/books/porters-handbook/book/#slow-patch-extra
> .if defined(MLTARGETS) - as I see MLTARGETS is defined everytime (at > least it contains heap2asm). So is the .if needed? Once upon a time there must have been a reason to introduce this .if:-) (And subsequently we obeyed the eternal rule: Never touch a running system;-) > And what if MLTARGETS isn't defined? The .tmp.sed file would be an > empty file (empty for loop), the sed will have an empty command list, > so nothing changes. You're right, so we can safely drop the .if. > Maybe this target can be simpler: append "request (every MLTARGETS)" > to targets.default. Or the order of requests is important? I think this would work - currently. But I see three issues: 1. Style: the request lines would be at the end of the file and hence away from the text that explains their meaning and caveats. 2. They would be out of scope of the if...elif...endif logic (which is evaluated by base/system/smlnj/installer/generic-install.sml) and things might break as a consequence. 3. If one would need or like to deselect targets (e.g. when introducing an OPTION MINIMAL), then using sed would pop up anyway. > Or more simpler (if the order is irrelevant): The order is irrelevant (as far as I can see). But the conditional structure of the targets file is not. > two files (mltargets-default and mltargets-everything) in FILESDIR > and append the option-specified content to targets.default. Apart from the problems with appending mentioned above, having seperate targets files in the port would (or at least: should) require synchronization with changes in upstream's targets file. > Or create two extra patch files [1]. patch files would reduce the synchronization problem, but would not remove it entirely. Maybe it's a question of style, whether MLTARGETS are explicit in the Makefile (make -V MLTARGETS) and work is done by the ${ECHO_CMD} and ${SED} lines in do-configure, or if MLTARGETS are scattered in patch files without requiring further work. I prefer the former. > do-configure-RECOMPILE-on: creating a file with ECHO_CMDs. > What about create a file (targets-recompile.customized) in FILESDIR > and copy it to config/targets.customized? My arguments from above regarding targets files don't apply in this case, so I agree. > And the comment can be in this file. So this target would be only one > line. I'd prefer to keep the comment in the Makefile, so one can get the full (admittedly: complicated) picture by reading it without diverting to other files. I understand your interest in simplifying the Makefile, but it is more than a build instruction for the machine, it's also documentation for humans (at least for the maintainer:). > ML*PATCHES*_CMD: the target is create a file list. You can use != > assignment in Makefile to set a value to a command's stdout. In this > case you don't need backticks in do-build. OK, I did it. The drawback was, that != replaces newlines with spaces, which broke PLIST generation with sed in pre-install-EVERYTHING-on. I fixed that using `tr " " "\n"`. But here's portlint's comment: WARN: Makefile: [146]: use of != in assignments is almost never a good thing to do. Try to avoid using them. See http://lists.freebsd.org/pipermail/freebsd-ports/2008-July/049777.html for some helpful hints on what to do instead. I'll keep the Makefile's version using != around in case portlint's complaint and the underlying reasoning are obsolete nowadays. But as the reasoning convinces me right now, I stick with ML*PATCHES*_CMD. > Does the install.sh use MLRUNTIMEPATCHES (and the others) environment > variable? I grepped everything but I couldn't find MLRUNTIMEPATCHES. (Lost in the jungle that grew in decades?-) .../lang/smlnj/files % fgrep MLRUNTIMEPATCHES * patch-config_install.sh:+ [ -n "$MLRUNTIMEPATCHES" ] && \ patch-config_install.sh:+ for p in $MLRUNTIMEPATCHES And that yields: .../work/smlnj-110.99.6.1/config % fgrep MLRUNTIMEPATCHES * install.sh: [ -n "$MLRUNTIMEPATCHES" ] && \ install.sh: for p in $MLRUNTIMEPATCHES > Uhhh, sorry for more work. Sorry for the Makefile being in such a bad (old) shape that it triggered your (helpful!) review. > If you don't have time can commit the patch from FTP and later (in > next year :) ) simplify. My view on the current state of this discussion is implemented here: <ftp://wrap7.free.de/pub/patch/smlnj.patch.20241229> MD5 (smlnj.patch.20241229) = 0dac72d60a8439c092c15832fe6ba3cb It includes a new file: files/targets-recompile.customized. Let me know if this patch is acceptable for you (and my poudriere testport outputs will follow). Thanks for your interest in this monster, and for your patience! Johannes
(In reply to Johannes 5 from comment #17) I think it's okay. I'll prepare the commit message and I commit it.
(In reply to Johannes 5 from comment #17) > I think this would work - currently. But I see three issues: Yes. I think there isn't perfect (best) solution, only solutions with shortcomings. > (Lost in the jungle that grew in decades?-) Didn't decompress everything :) > Sorry for the Makefile being in such a bad (old) shape that it > triggered your (helpful!) review. No problem. I like simplify the Makefiles :) If you'll have time and lust can try refactor the port. Committed, thanks!
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=e7fb3d8cf50264127c8058c251618ad310d71b23 commit e7fb3d8cf50264127c8058c251618ad310d71b23 Author: Johannes 5 <joemann@beefree.free.de> AuthorDate: 2024-12-30 13:39:44 +0000 Commit: Zsolt Udvari <uzsolt@FreeBSD.org> CommitDate: 2024-12-30 14:12:08 +0000 lang/smlnj: Update to 110.99.6.1 Add python:env and shebangfix to USES. Use target-OPTION-{on,off} instead of if-statements. Add targets-recompile.customized file instead of echo commands. Switch to DISTVERSION. Pet portfmt. Changelogs: https://www.smlnj.org/dist/working/110.99.6.1/110.99.6.1-README.html https://www.smlnj.org/dist/working/110.99.6/110.99.6-README.html https://www.smlnj.org/dist/working/2024.1/2024.1-README.html https://www.smlnj.org/dist/working/110.99.5/110.99.5-README.html https://www.smlnj.org/dist/working/2023.1/2023.1-README.html https://www.smlnj.org/dist/working/110.99.4/110.99.4-README.html https://www.smlnj.org/dist/working/2022.1/2022.1-README.html https://www.smlnj.org/dist/working/110.99.3/110.99.3-README.html https://www.smlnj.org/dist/working/2021.1/2021.1-README.html https://www.smlnj.org/dist/working/110.99.2/110.99.2-README.html https://www.smlnj.org/dist/working/110.99.1/110.99.1-README.html https://www.smlnj.org/dist/working/110.99/110.99-README.html PR: 283301 Reported by: Alexey Vyskubov <alexey@ocaml.nl> Approved by: submitter is maintainer lang/smlnj/Makefile | 158 +++++++++------------ lang/smlnj/distinfo | 98 ++++++------- lang/smlnj/files/do-patch-asdl_config.sh | 4 +- .../files/do-patch-asdl_src_asdlgen_Makefile.in | 6 +- .../files/do-patch-base_compiler_Parse_lex_ml.lex | 4 +- .../files/do-patch-base_compiler_Parse_lex_sml.lex | 4 +- .../do-patch-base_runtime_include_ml-unixdep.h | 2 +- .../files/do-patch-base_runtime_objs_makefile | 8 +- .../do-patch-base_runtime_objs_mk.amd64-freebsd | 6 +- .../do-patch-base_runtime_objs_mk.x86-freebsd | 16 +-- .../do-patch-smlnj-lib_JSON_json-util.sml (gone) | 29 ---- lang/smlnj/files/patch-config_install.sh | 22 +-- lang/smlnj/files/patch-config_unpack | 24 +++- .../smlnj/files/targets-recompile.customized (new) | 7 + lang/smlnj/pkg-plist | 12 +- 15 files changed, 187 insertions(+), 213 deletions(-)