NNPDF / pineappl

PineAPPL is not an extension of APPLgrid
https://nnpdf.github.io/pineappl/
GNU General Public License v3.0
13 stars 3 forks source link

Remove/fix `prettytable-rs` dependency #195

Closed cschwan closed 1 year ago

cschwan commented 1 year ago

Recently, ubuntu-latest was switched from ubuntu-20.04 to ubuntu-22.04, which seems to break pineappl. It segfaults, see https://github.com/NNPDF/pineappl/actions/runs/3646167214/jobs/6156975988. My bet is that it's not caused by pineappl itself but rather by a dependency.

alecandido commented 1 year ago

I can tell you that I could compile PineAPPL on my local Ubuntu 22.04, I did it many times. And run the CLI as well, of course.

cschwan commented 1 year ago

Yes, it's not quite that easy. What I noticed is that it started breaking when the switch happened, but it's still there with ubuntu-20.04.

It seems that pineappl [subcommand] --help is fine, while operations involving convolutions segfault: https://github.com/NNPDF/pineappl/actions/runs/3646494375/jobs/6157637600.

alecandido commented 1 year ago

I had a look to the logs, but it is highly non-trivial. Especially, I do not see what they all have in common: consider that also commands like pineappl obl are failing, and no convolution should be involved there.

Maybe the more precise statements is about grid loading: --help does not load any grid.

cschwan commented 1 year ago

True, that is even weirder.

cschwan commented 1 year ago

I can reproduce a segfault with Rust nightly, which segfaults in prettytable::Table::printstd - that makes sense with the observations above, they all print a table using prettytable-rs.

alecandido commented 1 year ago

I agree it matches the observation, but is the CI running with Nightly?

cschwan commented 1 year ago

Yes, it was. It works with stable, see here: https://github.com/NNPDF/pineappl/actions/runs/3646808485/jobs/6158307245. However, I think the problem is prettytable-rs, and Rust nightly is probably just more strict than stable (in some way I don't know). Since you can't produce segfaults in Rust unless you use unsafe, my bet is on

https://github.com/phsym/prettytable-rs/blob/e159600d999f55f49fa9141e1a014d776a67c836/src/lib.rs#L513-L518

which is the only piece marked 'unsafe'. I can confirm that this piece of code is called.

alecandido commented 1 year ago

Interestingly enough, the guy wrote:

// All this is a bit hacky. Let's try to find something else

on May 29, 2017, and it's still there.

It looks a good candidate to break with new versions. Essentially, it is recasting an immutable reference to a table to a mutable one, to perform a single operation. But that's an entry point for plenty of issues.

(and it is the only unsafe in the whole library in master, even though we should check the tag for the version used...)

alecandido commented 1 year ago

Ok, it seems like there is already an issue open mentioning it https://github.com/phsym/prettytable-rs/issues/145. Is this matching the usage in pineappl_cli? @cschwan

cschwan commented 1 year ago

Yes, I'm pretty sure that's the cause.

pinkforest commented 1 year ago

You can just bump prettytable-rs to 0.10 - I've pushed a release :)

cschwan commented 1 year ago

@pinkforest I'm already on it, thanks!