Ripgrep tests failures due to bstr update

Hi Guix,

I just talked to burntsushi on github about the failing ripgrep tests.

It looks like the version of bstr we are now using changed the

representation of bstrs and caused us test failures. Changing the

version of bstr used in ripgrep@12.1.1 to 0.2.12 should fix the problem.

See https://github.com/BurntSushi/ripgrep/discussions/1813

Thanks!

John

Hello,

John Soo <jsoo1@asu.edu> writes:

I disabled tests in ripgrep a few hours ago for the same reason. This is

not great, but I couldn't find another solution. In particular,

replacing rust-bstr-0.2 with a special rust-bstr-0.2.12 did not work

because regular rust-bstr-0.2 is still used as a transitive dependency.

Do you think of a better way to solve this?

Regards,

--

Nicolas Goaziou

Hello,

On Thu, Mar 04, 2021 at 09:38:39PM +0100, Nicolas Goaziou wrote:

Two possibilities seems to be possible here.

Upgrade rust-bstr-0.2 to be 0.2.12, possibly only upgrading needed

dependent packages.

Patch new ripgrep to understand the old bstr debug format in its tests.

This patch must be transitionnal while waiting for a compatible rust-bstr

package in Guix.

Disabling tests is bad. Disabling tests when we diverge from supported

upstream configuration (in this cas as we don't use recommended version

of deps) is in my opinion even worst.

François

Hello,

JOULAUD François via Bug reports for GNU Guix <bug-guix@gnu.org> writes:

Just to be clear, your are suggesting a downgrade, because currently,

Guix packages bstr 0.2.15. Downgrading is not trivial because bstr

0.2.15 is mandatory for bat 0.18.

We might provide both bstr 0.2.12 and 0.2.15. Note that 11 packages are

requiring bstr. Providing two bstr is only possible if none of those

packages are both among bat and ripgrep dependencies. I didn't check but

I'm not very optimistic.

I'll pass, sorry.

I'm not sure this kind of generalization helps here. Even if you may be

right in theory, looking at the number of "#:tests #f" among packages,

disabling tests is probably a necessary evil in practice.

Moreover, AFAICT, this incompatibility does not break functionality in

ripgrep. It just breaks its tests. Disabling them for a while does not

sound so bad, after all. Is it?

Regards,

--

Nicolas Goaziou

Hello,

On Mon, Mar 08, 2021 at 10:40:27PM +0100, Nicolas Goaziou wrote:

My mistake here. I thought upstream ripgrep was using a more recent

version than the packaged one. Downgrade could indeed be more work.

I'm not either. ;-)

I understand that. TBH I don't volunteer for the work either even if in

this specific case it seems to be a pretty basic substitution and one

that upstream will need to do anyway when it will upgrade to new bstr.

I think it is a necessary evil. I just fear that it could become an

initial reflex when dealing with library incompatibility.

But you're probably right that this is a discussion for an other venue.

Yes, in this specific case it seems to only break tests without breaking

anything else (and I trust upstream explanation on this one). But at the

same time, if it ought to be breaking something else you know don't have

the tests to try to catch it.

Regards,

François

Hey Nicolas,

I think the breaking tests do actually indicate breaking functionality in this case since there is a whole test suite dedicated to the bytestring representation. Maybe the fix could be using a later commit in which the later bstr is used. There are definitely unreleased commits in ripgrep.

Thanks!

John

Hello,

John Soo <jsoo1@asu.edu> writes:

I use ripgrep daily, and I haven't encountered a regression so far. Of

course, this proves nothing.

Latest ripgrep commit still requires bstr 0.2.12, so this is not an

option either.

Regards,

--

Nicolas Goaziou

Hey Nicolas,

I think the problem is that the version bound is too loose in cargo.toml upstream. What does the cargo.lock say on the current master? I am beginning to think we need to follow the cargo.lock to resolve dependencies first if it exists.

Thanks,

John

John Soo <jsoo1@asu.edu> writes:

It's 0.2.12 in Cargo.toml and 0.2.14 in Cargo.lock.

See https://github.com/BurntSushi/ripgrep/blob/master/Cargo.lock

Regards,