[lrslib] Add LibraryProduct for liblrsnash

[bzinberg]

Adds a LibraryProduct for liblrsnash, which will allow this JLL to be used by JuliaPolyhedra/LRSLib.jl.

x-ref: JuliaPolyhedra/LRSLib.jl#39

[bzinberg]

@giordano, all the builds succeed on my machine, though it looks like some of them failed in CI. Any idea what's happening?

[giordano]

all the builds succeed on my machine

What version of Julia and BinaryBuilder are you using? We use Julia v1.6.0 with the environment in the .ci/ directory, I'd recommend you do instantiate that one if you want to debug the issue.

Any idea what's happening?

At a quick glance,
https://github.com/JuliaPolyhedra/lrslib/blob/d8b723a2c315614979a8354f9e768d273d14a215/lrsnashlib.h#L70
https://github.com/JuliaPolyhedra/lrslib/blob/d8b723a2c315614979a8354f9e768d273d14a215/lrsnashlib.c#L51
look like problematic. However I don't have the time to dig this further.

Edit: or maybe the issue is that the global variable in the header file is being defined twice in two different object files. It's just a completely bad idea to define variables in header files (without being extern at least).

[bzinberg]

What version of Julia and BinaryBuilder are you using? We use Julia v1.6.0 with the environment in the .ci/ directory, I'd recommend you do instantiate that one if you want to debug the issue.

Will do, thanks for the pointer. I'm currently on Julia 1.5.3 and some less-than-current version of BinaryBuilder.

At a quick glance,

Yeah, it looked to me like clang's complaint was probably valid and just being let slide by the GNU toolchain. I'm going to first try just setting the mac and freebsd toolchains to gnu and see if that makes CI pass, and move on to the linkage bug if that doesn't work.

[giordano]

You may have missed the edit:

Edit: or maybe the issue is that the global variable in the header file is being defined twice in two different object files. It's just a completely bad idea to define variables in header files (without being extern at least).

I think clang is being more strict, but also more correct. It looks like the source code is simply wrong.

[bzinberg]

Edit: or maybe the issue is that the global variable in the header file is being defined twice in two different object files. It's just a completely bad idea to define variables in header files (without being extern at least).

I think clang is being more strict, but also more correct. It looks like the source code is simply wrong.

Yeah, I'd never seen a non-static non-extern global variable declaration in a header file before (though, not an experienced C programmer), had to look up what it means. External linkage and static storage duration. I still want to see if using the GNU toolchain makes CI pass, though I agree that it would only be passing due to undefined behavior.

[bzinberg]

Switching to GNU toolchain fixed CI. I think pending the above comment about LDFLAGS, this may be ready to merge?

[bzinberg]

@giordano, are we ready to merge now or do you think this PR needs to wait on JuliaPolyhedra/lrslib#2? If needs to wait, then @blegat I suggest we merge JuliaPolyhedra/lrslib#3 so that the migration to BinaryBuilder isn't delayed.

[giordano]

Ok, let's go with this one, but a proper fix upstream rather than ignoring the issue would be better 🙂

[bzinberg]

Thanks! And a fix is under way in JuliaPolyhedra/lrslib#3!