Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release-fast and release-small profiles and a page on packaging in the docs. #4470

Merged
merged 2 commits into from
Mar 24, 2023

Conversation

tertsdiepraam
Copy link
Member

@tertsdiepraam tertsdiepraam commented Mar 6, 2023

Inspired by #4436 and #747.

In the comments of #4436, I did an exploration of the binary size by trying different settings in cargo. The conclusion was that we can get the binary size down by a lot (even smaller than GNU), but that this will make bug reports less useful. Therefore, we should leave the exact configuration up to package maintainers. To document that, I've written a page on packaging this project in the docs. This page explains the best practices for the package name (since we had some issues there) and how to configure the package.

Additionally, to quickly see what our best performance and smallest binary size is, I've created the release-fast and release-small profiles in addition to the standard release profile. The release profile also has lto enabled, because that (probably) gives a performance boost and reduces the binary quite a bit, without removing any debug info, but compilation takes a bit slower. (I haven't benchmarked the performance and compile time though)

The change in stdbuf is because it was using the PROFILE env variable, which can only be release or debug and does not become, for example, release-fast and was therefore failing to compile.

@sylvestre, since you're packaging expert; is there anything else we should mention on the packaging page?

@tertsdiepraam
Copy link
Member Author

tertsdiepraam commented Mar 6, 2023

Btw, we might need to advertise these alternative profiles a bit more after this. It would be nice to not have people complain about binary size for once when this project makes the rounds again 😄

@tertsdiepraam tertsdiepraam force-pushed the release-fast-and-small branch 2 times, most recently from f2759e5 to ea955fd Compare March 6, 2023 22:30
@tertsdiepraam
Copy link
Member Author

Oh great, I now remember why stdbuf was using that variable in the first place: #2843. I'll look into a solution tomorrow.

@github-actions
Copy link

github-actions bot commented Mar 6, 2023

GNU testsuite comparison:

GNU test failed: tests/tail-2/inotify-dir-recreate. tests/tail-2/inotify-dir-recreate is passing on 'main'. Maybe you have to rebase?

@sylvestre
Copy link
Contributor

I think it would be nice to test both of them in the CI, don't you think ? :)

@tertsdiepraam tertsdiepraam marked this pull request as draft March 7, 2023 09:03
@tertsdiepraam
Copy link
Member Author

Yes, I'll add that too!

@tertsdiepraam tertsdiepraam force-pushed the release-fast-and-small branch from ea955fd to 9d57378 Compare March 15, 2023 13:32
@tertsdiepraam tertsdiepraam marked this pull request as ready for review March 15, 2023 13:33
@tertsdiepraam tertsdiepraam force-pushed the release-fast-and-small branch from 9d57378 to 8c19ee3 Compare March 15, 2023 13:35
@tertsdiepraam
Copy link
Member Author

On second thought, maybe it's not that helpful to add in the CI? Apart from the anomaly with stdbuf, the profile should have very little effect on whether or not it compiles.

@sylvestre sylvestre merged commit ed4222e into uutils:main Mar 24, 2023
@sylvestre
Copy link
Contributor

ok, you are probably correct :)

@tertsdiepraam
Copy link
Member Author

@sylvestre I think this causes problems after all. Lot's of builds failing because of stdbuf. We should probably revert this.

@sylvestre
Copy link
Contributor

@tertsdiepraam
Copy link
Member Author

Nice! And that's just the lto change too.

On second thought, maybe it's not that helpful to add in the CI? Apart from the anomaly with stdbuf, the profile should have very little effect on whether or not it compiles.

I'm not so sure about this anymore, it would be interesting to add it to the CI just for tracking the size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants