Skip to content

source: Switch to xz for packages and tools where possible#361

Closed
diizzyy wants to merge 3 commits intolede-project:masterfrom
diizzyy:master
Closed

source: Switch to xz for packages and tools where possible#361
diizzyy wants to merge 3 commits intolede-project:masterfrom
diizzyy:master

Conversation

@diizzyy
Copy link
Contributor

@diizzyy diizzyy commented Oct 1, 2016

Adds a slightly higher compression level to xz by default which roughly raises memory usage from 100MiB to about 200MiB during compression, about 10MiB for decompression. (Source: xz manpage)
Changes git packages to xz
Removes mirror checksums to avoid mismatches with tarballs as they differ due to checkout date and time of files.
Changes a few source tarballs to xz if available upstream.

Signed-off-by: Daniel Engberg daniel.engberg.lists@pyret.net

@diizzyy
Copy link
Contributor Author

diizzyy commented Oct 1, 2016

Not sure if PKG_RELEASE should be bumped or not.

@ignisf
Copy link
Contributor

ignisf commented Oct 2, 2016

openwrt/packages#149

Incrementing PKG_RELEASE has been a bit hazy to me, too.

Has the PKG_RELEASE convention been documented anywhere in the wiki? I've forgotten to bump the PKG_RELEASE a few times and my patches have been merged anyways.

Is the linked convention adopted by LEDE?

If it is, is it OK if I added it to https://wiki.openwrt.org/doc/devel/packages?

@neocturne
Copy link
Member

Generally, PKG_RELEASE should be incremented when the resulting binary package will change (either the contained files or the metadata like dependencies), so opkg will notice the update.

For build fixes or changing to another equivalent source URL or tarball (like here) no increment should be done, as there is nothing for opkg to update.

Other comments:

  • Switching downloaded tarballs to .xz seems reasonable to me
  • The tarball for the package xz should be kept as bz2, so xz can be built when it is not available on the host system (bzip2 is checked as a prerequisite of LEDE, xz isn't; adding xz to the prereqs might be an alternative)

How much smaller do the tarballs get with -7e? The effect will only be noticable for huge tarballs, but in my experience it's not worth the increased time needed for the compression of big files.

@diizzyy
Copy link
Contributor Author

diizzyy commented Oct 2, 2016

I think it's resonable to expect xz support these days, I did try to find how prereqs were generated but failed, if someone can give me pointers I'll add that too.

Most base packages see a slightly smaller size (it's usually within 1-2KiB) however it's more beneficial for external packages which can be quite a bit larger and I don't see much of a difference in CPU time for base packages given their overall size. @jow- is also planning to make git packages reproducible (for mirroring with checksums) and if we can save space and bandwidth I think it's worth the slight overhead it may inflict.

@diizzyy
Copy link
Contributor Author

diizzyy commented Oct 2, 2016

I've added xz-utils (xz) to make prereq

Changes git packages to xz
Removes mirror checksums to avoid mismatches with tarballs as they differ due to checkout date and time of files.
Changes a few source tarballs to xz if available upstream.

Signed-off-by: Daniel Engberg <daniel.engberg.lists@pyret.net>
Add xz-utils to make prereq

Signed-off-by: Daniel Engberg <daniel.engberg.lists@pyret.net>
Adds a slightly higher compression level to xz by default which roughly raises memory usage from 100MiB to about 200MiB during compression, about 10MiB for decompression. (Source: xz manpage)

Signed-off-by: Daniel Engberg <daniel.engberg.lists@pyret.net>
PKG_MIRROR_MD5SUM:=b80d6ddcd6f6925e2f5b80775bb96f3e
PKG_CAT:=zcat
PKG_MIRROR_MD5SUM:=
# PKG_CAT:=zcat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a commented out line here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't find any documentation or figure out what it actually did, there are more occurrences of stray lines in package Makefiles so I'd rather do a tree-wide cleanup afterwards.

PKG_SOURCE_VERSION:=d333fc232d7e5ae3370080d5d6f7d88ea9c6b3a1
PKG_MIRROR_MD5SUM:=8145d4dd9b46face26121f27ad60cc1c
PKG_MIRROR_MD5SUM:=
HOST_BUILD_DIR=$(BUILD_DIR_HOST)/$(PKG_NAME)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the whole line if it is not needed and explain this change in the commit message. this is unrealated to changing the compression to xz

Copy link
Contributor Author

@diizzyy diizzyy Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an issue because git doesn't date and time-stamp files during cloning so each package will be different unless you clone at the exact same time and at the exact same speed. As someone who doesn't have access to the "fallback" download server it's impossible/doesn't make any sense to add a checksum for a package that's more than likely mismatch compared to what's going to be uploaded if any at all. There are more Makefiles in the tree which have empty PKG_MIRROR_MD5SUM vars so I figured it would be more consistent to leave those empty rather than removing them.

PKG_SOURCE_SUBDIR:=$(PKG_NAME)-$(PKG_VERSION)
PKG_MIRROR_MD5SUM:=
PKG_CAT:=zcat
# PKG_CAT:=zcat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not delete the line ?

Copy link
Contributor Author

@diizzyy diizzyy Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See first comment

PKG_SOURCE_VERSION:=8dce53297966b31b6c70a7a03c2433978dd9f288
#PKG_MIRROR_MD5SUM:=50ca3c763ee21ee213addd17cf1c1b86
PKG_MIRROR_MD5SUM:=
HOST_BUILD_DIR=$(BUILD_DIR_HOST)/$(PKG_NAME)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove it if it not needed

Copy link
Contributor Author

@diizzyy diizzyy Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See second comment

@diizzyy
Copy link
Contributor Author

diizzyy commented Oct 4, 2016

Due to my silly mistake not using branches makes this much harder to fix. Continued (and fixed) in #375

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.

4 participants