Byteyarn fails to build on i686 and s390x

[blinxen]

gix-attributes 0.19.0 introduced the usage of byteyarn to optimize the memory footprint. This is problematic because of the following issue mcy/strings#3. Is it possible to revert the commit and reuse kstring until the byteyarn issue is resolved?

[Byron]

Thanks for reporting!

I'd love to prevent such regression in future by having CI tests run these platforms. Could you help setting these up here (even if it's by pointing me at a sample CI setup that could be copied from?).

Then I'd like to wait for a response on the related issue on byteyarn to see when or if a fix could be happening. If there will be no fix or it takes more than a week or so, I will definitely revert to kstring like suggested.

[Byron]

"feedback requested" to indicate the waiting on mcy/strings#3.

[blinxen]

Could you help setting these up here (even if it's by pointing me at a sample CI setup that could be copied from?).

Sure! Will take a look this weekend.

[epage]

btw I have a larger list of alternatives at https://github.com/rosetta-rs/string-rosetta-rs (byteyarn hasn't even made it onto the list ...)

[Byron]

Thanks for the hint! Maybe there could even be another column in the primary table to indicate if byte-strings are supported. The issue with kstring also was that hackery was necessary to use it with byte strings, and any byteyarn replacement should it be necessary would ideally come with a way to not assume UTF-8 encoding of its contents.

I'd understand if that is out-of-scope for the comparison though.

[blinxen]

I looked into it and your CI already runs tests on 32-bit machines --> https://github.com/Byron/gitoxide/blob/main/.github/workflows/ci.yml#L90

But byteyarn has 99% of its tests as doctests. Which means that you need to also test doctests in your CI to notice this kind of behavior. Before I do the work, do you also want to test for doctests in your CI? I can prepare a PR if you want :D

[Byron]

But byteyarn has 99% of its tests as doctests. Which means that you need to also test doctests in your CI to notice this kind of behavior. Before I do the work, do you also want to test for doctests in your CI? I can prepare a PR if you want :D

Thanks for looking into this! However, I think I see my mistake now: I assumed that gitoxide tests could have caught this issue, but that's not the case. From a CI point of view, everything works even on 32 bit, even though one must say that only a couple of tests run that are not truly representative. Thus the current 32bit test might yield a false sense of safety. s390x might be a different topic alltogether (I don't know it) - maybe that should be added. Also, it's not my intend to run byteyarn tests on gitoxide's CI.

With that said, maybe s390x could be added if it makes sense just to run.

On another note, I think it's time to roll back to kstring very soon, maybe that alleviates all other concerns of this issue as well even though improved 32 bit testing would definitely not hurt.

[decathorpe]

Note that cross-compiled environments apparently do not support compiling / running doctests - something I've come across in the regex* crates as well: rust-lang/regex#1041 (comment)

[Byron]

Thanks for the head's up! gitoxide doesn't have many doc-tests yet - I think there is only one crate that has them, gix-config, so it's probably not much of an issue at first and still a step up to run at least some of the tests on more platforms.

I always have trouble wrapping my head around cross-compilation setups which is probably why it's not done here yet. Admittedly, I haven't really tried either.

[NobodyXu]

I'd recommend cargo-zigbuild for cross compiling (we used it in cargo-binstall) and tokio uses taiki-e/setup-cross-toolchain-action for cross test.

[Byron]

It should be easy enough to get that going, thanks for the hint. Even though I still don't feel like going through the pain which I expect it to be nonetheless, maybe I am too pessimistic about it though 😅.

[Byron]

You can expect a release on October 22nd now that the byteyarn addition has been reverted. I'd love to see the 32 bit CI improved as well, but feel like I have no bandwidth for it right now.