Add libtommath 1.3.0

[cyanogilvie]

Add libtommath 1.3.0

[bgilbert]

Thanks for the PR!

[cyanogilvie]

After extensive hacking the build and tests succeed on Windows MSVC and clang-cl, so I've enabled them in the CI

[bgilbert]

A bunch of small notes, but looks good generally. Thanks for your work on this!

[bgilbert]

This seems surprising and inconsistent as a default. If it's needed because tests require the static library, it seems better to build the library with both_libraries() (or possibly conditionalize on the tests option using build_target()) instead.

[bgilbert]

...in which case, you should probably also do something like this:

wrapdb/subprojects/packagefiles/libarchive/libarchive/meson.build

Lines 176 to 181 in 22e2211

	# default_both_libraries requires Meson >= 1.6
	if get_option('default_library') == 'static'
	libarchive = libarchive_both.get_static_lib()
	else
	libarchive = libarchive_both.get_shared_lib()
	endif

[cyanogilvie]

Can we go with the alternative: conditionalize building and running the tests and set default_library=both in the CI? Hard-coding both_libraries internally and then re-implementing the intent of the default_library standard option seems brittle (the above code would defeat a consumer's intent if they explicitly specify default_library=both - handing them a lib rather than the both_libs they would expect). This way the CI works and runs the tests on all platforms and the consumer gets what they expect with the least surprising internal implementation.

  libtommath_for_tests = libtommath
  if cc.get_id() in ['msvc', 'clang-cl']
    if get_option('default_library') == 'shared'
      error('Static library build is required to run tests with MSVC/clang-cl')
    endif
    libtommath_for_tests = libtommath.get_static_lib()
  endif

I would have liked to use libtommath.get_static_lib().found() to more directly test the relevant requirement, but it seems that libraries() doesn't return both_libs if only one is selected (and so the get_*_library() methods are missing, or only the corresponding method exists).

[bgilbert]

Mild preference for building these up beforehand, which is idiomatic, rather than using ternaries here.

[cyanogilvie]

I expressed these as a ternaries to make reasoning about the scope of their effect easier - like that it cannot affect anything other than the link_args of this target, whereas if it is coupled through global state by constructing it as a conditionally defined variable earlier it would require searching for other uses of that variable to understand its scope, but I'll rework it through a variable if the ternary clashes with the idiomatic style of this project?

[bgilbert]

What's happening here?

[cyanogilvie]

I have no idea. It dates back to a commit in 2003 with the message added libtommath-0.17 which I take to mean that this libtommath repo was extracted from some other project that imported libtommath from elsewhere (libtomcrypt?), or that back then libtommath development didn't use VC, so wasn't able to trace the intent.

[bgilbert]

Is this intentionally doubled?

[cyanogilvie]

Only insofar as it replicates what upstream's makefile does. My best guess is that there is some build stability hysteresis issue that produces different results if the output artifacts exist already

[bgilbert]

Seems like there's a lot of unnecessary code to move files around. Why not just write the deterministic header directly into bn.tex (or into tempfile.NamedTemporaryFile()), copy input_file into it afterward, and set its timestamp from input_file?

[cyanogilvie]

Re this and the other review comments on this script: this is a python port of upstream's doc build rules:

docs: manual

#LTM user manual
mandvi: bn.tex
    cp bn.tex bn.bak
    touch --reference=bn.tex bn.bak
    (printf "%s" "\def\fixedpdfdate{"; date +'D:%Y%m%d%H%M%S%:z' -d @$$(stat --format=%Y bn.tex) | sed "s/:\([0-9][0-9]\)$$/'\1'}/g") > bn-deterministic.tex
    printf "%s\n" "\pdfinfo{" >> bn-deterministic.tex
    printf "%s\n" "  /CreationDate (\fixedpdfdate)" >> bn-deterministic.tex
    printf "%s\n}\n" "  /ModDate (\fixedpdfdate)" >> bn-deterministic.tex
    cat bn.tex >> bn-deterministic.tex
    mv bn-deterministic.tex bn.tex
    touch --reference=bn.bak bn.tex
    echo "hello" > bn.ind
    latex bn ${silent_stdout}
    latex bn ${silent_stdout}
    makeindex bn
    latex bn ${silent_stdout}

#LTM user manual [pdf]
manual: mandvi
    pdflatex bn >/dev/null
    sed -b -i 's,^/ID \[.*\]$$,/ID [<0> <0>],g' bn.pdf
    mv bn.bak bn.tex
    rm -f bn.aux bn.dvi bn.log bn.idx bn.lof bn.out bn.toc

Which seem to contain a lot of voodoo in pursuit of deterministic builds, but I honestly don't understand the intent of each bit well enough to reason about which side effects are being relied upon (and the commit history didn't shed any light on it), so I instead aimed to replicate the process and side-effects as exactly as possible.

My python is pretty rusty (I last did any substantial work in python over 5 years ago, and even then not much), so I used AI for the initial port and then checked it as best I can, which probably hasn't helped matters.

For my use case I don't care about deterministic builds (especially for an optional documentation target). That seems like a core focus of other build systems like bazel, and I've seen it mentioned for meson but I don't know what the official stance is towards deterministic builds here. It's curious that libtommath seemed to work so hard on making this output deterministic 23 years go, long before I'd heard anything about deterministic builds.

[bgilbert]

These are all written to the build directory. Is it important to clean them up?

[cyanogilvie]

I would certainly leave that to the normal build dir handling, but since upstream's rules explicitly cleaned up these files separately from its clean target (which includes a superset of them), I interpreted it as meaning that their presence during subsequent builds would affect the determinism so I cargo culted it.

[cyanogilvie]

Applied the suggested changes or clarified some reasoning. Some issues still open

[cyanogilvie]

I expressed these as a ternaries to make reasoning about the scope of their effect easier - like that it cannot affect anything other than the link_args of this target, whereas if it is coupled through global state by constructing it as a conditionally defined variable earlier it would require searching for other uses of that variable to understand its scope, but I'll rework it through a variable if the ternary clashes with the idiomatic style of this project?

[cyanogilvie]

Re this and the other review comments on this script: this is a python port of upstream's doc build rules:

docs: manual

#LTM user manual
mandvi: bn.tex
    cp bn.tex bn.bak
    touch --reference=bn.tex bn.bak
    (printf "%s" "\def\fixedpdfdate{"; date +'D:%Y%m%d%H%M%S%:z' -d @$$(stat --format=%Y bn.tex) | sed "s/:\([0-9][0-9]\)$$/'\1'}/g") > bn-deterministic.tex
    printf "%s\n" "\pdfinfo{" >> bn-deterministic.tex
    printf "%s\n" "  /CreationDate (\fixedpdfdate)" >> bn-deterministic.tex
    printf "%s\n}\n" "  /ModDate (\fixedpdfdate)" >> bn-deterministic.tex
    cat bn.tex >> bn-deterministic.tex
    mv bn-deterministic.tex bn.tex
    touch --reference=bn.bak bn.tex
    echo "hello" > bn.ind
    latex bn ${silent_stdout}
    latex bn ${silent_stdout}
    makeindex bn
    latex bn ${silent_stdout}

#LTM user manual [pdf]
manual: mandvi
    pdflatex bn >/dev/null
    sed -b -i 's,^/ID \[.*\]$$,/ID [<0> <0>],g' bn.pdf
    mv bn.bak bn.tex
    rm -f bn.aux bn.dvi bn.log bn.idx bn.lof bn.out bn.toc

Which seem to contain a lot of voodoo in pursuit of deterministic builds, but I honestly don't understand the intent of each bit well enough to reason about which side effects are being relied upon (and the commit history didn't shed any light on it), so I instead aimed to replicate the process and side-effects as exactly as possible.

My python is pretty rusty (I last did any substantial work in python over 5 years ago, and even then not much), so I used AI for the initial port and then checked it as best I can, which probably hasn't helped matters.

For my use case I don't care about deterministic builds (especially for an optional documentation target). That seems like a core focus of other build systems like bazel, and I've seen it mentioned for meson but I don't know what the official stance is towards deterministic builds here. It's curious that libtommath seemed to work so hard on making this output deterministic 23 years go, long before I'd heard anything about deterministic builds.

[cyanogilvie]

I have no idea. It dates back to a commit in 2003 with the message added libtommath-0.17 which I take to mean that this libtommath repo was extracted from some other project that imported libtommath from elsewhere (libtomcrypt?), or that back then libtommath development didn't use VC, so wasn't able to trace the intent.

[cyanogilvie]

Only insofar as it replicates what upstream's makefile does. My best guess is that there is some build stability hysteresis issue that produces different results if the output artifacts exist already

[cyanogilvie]

I would certainly leave that to the normal build dir handling, but since upstream's rules explicitly cleaned up these files separately from its clean target (which includes a superset of them), I interpreted it as meaning that their presence during subsequent builds would affect the determinism so I cargo culted it.