imneme / pcg-c

PCG — C Implementation
Apache License 2.0
361 stars 59 forks source link

pcg-c fails to build unless 128-bit ops are supported (as e.g. on NetBSD/powerpc) #32

Open he32 opened 1 year ago

he32 commented 1 year ago

This popped up on my radar due to a build failure on NetBSD/powerpc. This platform does not have native support for 128-bit integer operations.

gcc -O2 -D_FORTIFY_SOURCE=2 -O3 -std=c99 -I../include  -c -o pcg-global-32.o pcg-global-32.c
In file included from pcg-global-32.c:33:
../include/pcg_variants.h:2178:33: error: 'PCG_STATE_SETSEQ_64_INITIALIZER' undeclared here (not in a function)
 2178 | #define PCG32_INITIALIZER       PCG_STATE_SETSEQ_64_INITIALIZER
      |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pcg-global-32.c:35:38: note: in expansion of macro 'PCG32_INITIALIZER'
   35 | static pcg32_random_t pcg32_global = PCG32_INITIALIZER;
      |                                      ^~~~~~~~~~~~~~~~~
gmake[1]: *** [<builtin>: pcg-global-32.o] Error 1

Inspecting include/pcg_variants.h reveals that PCG_HAS_128BIT_OPS ends up as undefined on this platform, due to lack of __SIZEOF_INT128__ being defined, and hence PCG_STATE_SETSEQ_64_INITIALIZER also ends up being undefined, as it's only defined if PCG_HAS_128BIT_OPS is defined.

rdebath commented 1 year ago

You don't seem to have checked out the current version as this was fixed in 2017.

he32 commented 1 year ago

You don't seem to have checked out the current version as this was fixed in 2017.

That's entirely possibly. I'm referring to the latest published C version, 0.94, which appears to have this issue. Has there been a later release of that code base? https://www.pcg-random.org/download.html appears to say "no".

rdebath commented 1 year ago

Ah, I see, so the zip on the website needs updating or pointing at this repository for the zip. I didn't notice because I looked at the code here and then downloaded the most recent version.

For example: https://github.com/imneme/pcg-c/archive/refs/heads/master.zip

he32 commented 1 year ago

Ah, I see, so the zip on the website needs updating or pointing at this repository for the zip. I didn't notice because I looked at the code here and then downloaded the most recent version.

For example: https://github.com/imneme/pcg-c/archive/refs/heads/master.zip

I come at this problem working on the packaging system originating at NetBSD, pkgsrc. Having a clearly identifyable release is vastly preferable to "just use the tip of the head of this repository", as the latter is clearly not uniquely identifyable, and prone to change as time passes.

I hope you're not allergic to making a full release? At the minimum, a tag should be created in the repository to mark exactly which point corresponds to this release, but of course a "full" 0.95 release would be preferable.

rdebath commented 1 year ago

This isn't my repo, so like you I can only suggest to @imneme to label up a release.

However, for you now, Git is designed to be able to uniquely identify any commit. It is usual to choose a commit for your submodule and freeze your reference at that commit.

Also for the commit of the current master Github will create a zip if you ask for the URL https://github.com/imneme/pcg-c/archive/83252d9c23df9c82ecb42210afed61a7b42402d7.zip They will also provide you with zips (if you need them) for any unique commit.