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

barebox: readd lzop-native #133

Merged
merged 1 commit into from
May 7, 2024
Merged

Conversation

Emantor
Copy link
Member

@Emantor Emantor commented Apr 30, 2024

In scarthgap, lzop has been readded to poky with commit 07006c486722 ("Revert "lzop: remove recipe from oe-core""), readd it to the barebox depends to reallow lzop compression.

This reverts commit 38ada68.

@ejoerns
Copy link
Member

ejoerns commented Apr 30, 2024

@Emantor Do we really still need it for newer barebox integrations?

@a3f
Copy link
Member

a3f commented Apr 30, 2024

The upstream configs have been switched to LZ4 in https://lore.barebox.org/barebox/20221206112614.2612071-1-m.felsch@pengutronix.de/#t, but LZO remains a better compression choice if available. ZSTD would be a nice replacement for LZO, but the decompressor turned out to be quite heavy: https://lore.barebox.org/barebox/20220713100922.1880282-1-a.fatoum@pengutronix.de/

@Emantor
Copy link
Member Author

Emantor commented Apr 30, 2024

@Emantor Do we really still need it for newer barebox integrations?

Regardless whether we have switched over integrations since the removal, LZO is currently supported within upstream barebox and as the poky commit notes still the best time/compression compromise on lower end ARM systems. I don't see a reason to not support LZO.

@ejoerns
Copy link
Member

ejoerns commented Apr 30, 2024

@Emantor Do we really still need it for newer barebox integrations?

Regardless whether we have switched over integrations since the removal, LZO is currently supported within upstream barebox and as the poky commit notes still the best time/compression compromise on lower end ARM systems. I don't see a reason to not support LZO.

It's not an option to add the dependency for cases where it's actually used?

Also, note that the recipe is a bit deprecated anyway.

nit: the referenced commit is a poky commit, not an oe-core commit

In scarthgap, lzop has been readded to poky with commit 07006c486722
("Revert "lzop: remove recipe from oe-core""), readd it to the barebox
depends to reallow lzop compression.

This reverts commit 38ada68.

Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
@Emantor
Copy link
Member Author

Emantor commented Apr 30, 2024

@Emantor Do we really still need it for newer barebox integrations?

Regardless whether we have switched over integrations since the removal, LZO is currently supported within upstream barebox and as the poky commit notes still the best time/compression compromise on lower end ARM systems. I don't see a reason to not support LZO.

It's not an option to add the dependency for cases where it's actually used?

Yes, I already did that. From a usability perspective I want to readd this to the upstream recipe since especially older kirkstone BSPs are prone to have this set and they will stumble upon this during a scarthgap bump. Since LZO is supported both in poky/oe-core and barebox, I don't see a reason to fix this for every BSP.

Also, note that the recipe is a bit deprecated anyway.

In that case we should add a deprecation note and have a removal plan.

nit: the referenced commit is a poky commit, not an oe-core commit

Fixed this with a force push.

@ejoerns ejoerns merged commit e733426 into pengutronix:master May 7, 2024
2 checks passed
@Emantor Emantor deleted the topic/readd-lzop branch May 8, 2024 06:41
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.

3 participants