(GCI) Added recipe for sys-fs/squashfs_tools #948
Conversation
| " | ||
|
|
||
| DEBIAN_REV="3" | ||
| SOURCE_URI_2="http://http.debian.net/debian/pool/main/s/squashfs-tools/squashfs-tools_$portVersion-$DEBIAN_REV.debian.tar.xz" |
There was a problem hiding this comment.
It would be slightly better to use the canonical name:(httpredir.debian.org instead of http.debian.net):
SOURCE_URI_2="http://httpredir.debian.org/debian/pool/main/s/squashfs-tools/squashfs-tools_$portVersion-$DEBIAN_REV.debian.tar.xz"
| COPYRIGHT="2002-2014 Philip Lougher" | ||
| LICENSE="GNU GPL v2" | ||
| REVISION="1" | ||
| SOURCE_URI="https://prdownloads.sf.net/squashfs/squashfs$portVersion.tar.gz" |
There was a problem hiding this comment.
I think we should avoid the old "prdownloads.sf.net" name and, instead, use "downloads.sf.net" or "downloads.sourceforge.net":
SOURCE_URI="https://downloads.sf.net/squashfs/squashfs$portVersion.tar.gz"
| read-only filesystem use, archival use (i.e. in cases where a .tar.gz file \ | ||
| may be used, and in constrained block device/memory systems (e.g. embedded \ | ||
| systems) where low overhead is needed." | ||
| HOMEPAGE="http://squashfs.sourceforge.net" |
There was a problem hiding this comment.
Please add a trailing slash:
HOMEPAGE="http://squashfs.sourceforge.net/"
|
Alright, I'll fix it up right away! |
|
I pushed the amended commit out. For some reason, it still shows up in the same place as the old one. But it has been changed. |
|
Do you have lib:liblzma on x86_gcc2? BTW, I noticed that several patches were already available on Gentoo, so we could do something similar to what we have for tiff-3.9.7. ... and then: The only problem is that the patches do not apply yet. Maybe I need to change "-p1". |
|
I actually grabbed three of the four other patches from the debian patchset. So we could apply them manually from |
|
The reason I put them all in |
|
I think we should use as much external patches as possible. BYW, doesn't your recipe depend on lib:liblzma ? That lib is provided by xz_utils which does not build on x86_gcc2. @waddlesplash, did you build the squashfs_tools on x86_gcc2? |
|
Oh, I didn't know |
|
Since Debian has the remaining patches in squashfs-tools_4.3-3.debian.tar.xz we can probably add something like this in our PATCH function: |
|
Actually, I'd skip the first one ( |
|
We could add support for x86 secondary arch and define LIBLZMA like this: Then we would use ${LIBLZMA+lib:$LIBLZMA} in REQUIRES and ${LIBLZMA+devel:$LIBLZMA} in BUILD_REQUIRES. |
|
Well, I'll try to get xz-utils to build on x86 first, since it'd be nice to have them there anyway. |
|
Do you mean "on x86_gcc2" ? |
Yes, sorry, I tend to conflate the two. |
|
Wait, I think I messed up somewhere |
|
Oh yeah, I see it now. I'll have to use your method. |
|
Just to give you an update, turns out, the sources don't quite build with GCC2. Investigating the issue. |
|
Alright, now everything seems to be in order. |
|
Oops, didn't mean to press that 😅 |
|
@fbrosson I'm personally not so happy with a ton of URL for patches which won't change forever, because the upstream project stalled. It's adding a dependency on Gentoo servers for no good reason, simply packing all patches in a gentoo patchset in patches directory would be better and more maintainable. |
|
Alright, I like this version a lot better. I do agree with @korli about verbosity, but I'm new here, so I don't really know what the common practice is. |
|
@hermord, I love your current recipe! (I'll test it asap.) @korli, I'm sorry, I don't think that adding copies of patches that are already available somewhere would be a good idea. And imho the dependency on Gentoo and Debian is not a problem. Moreover, since we have checksums, we would detect any change or removal, should that ever happen. |
|
Yes, I just expect your opinion to not become a rule. |
|
I'm rather conflicted on the remote-patches idea myself; I'm tending towards @korli's stance on it but I haven't really thought about it. But we don't need to do it all that often, so I don't think it'll become a problem... |
|
@korli: Sure, this won't become a rule because there are very few recipes where this can happen. @waddlesplash: I think you have found a nice consensus. @hermord: squashfs_tools builds like a charm on x86 and x86_64. Will try it on x86_gcc2 right now. All: Shall we validate the GCI task right now but keep the PR open for 1 or 2 days to do a little polishing (e.g. see if we can get less fuzzy patches by either sorting them or rebasing the patches) and maybe add support for building for x86 secondary arch? Thanks to all of you! |
|
That sounds OK to me. |
|
Alright, so I got rid of two patches that are not needed on Haiku (they have to do with xattrs and linux-only includes). I also changed my own patches a bit so that they don't mess with offsets. Sadly, Debian patches still have wrong offsets and that doesn't seem to depend on ordering. |
OK; so we should fetch all of those and make a .patchset ourselves out of them. |
| SOURCE_DIR="squashfs$portVersion/squashfs-tools" | ||
|
|
||
| DEBIAN_REV="3" | ||
| SOURCE_FILENAME_2="squashfs-tools_$portVersion-$DEBIAN_REV.debian.tar.xz" |
There was a problem hiding this comment.
I would recommend avoiding all-caps for custom variables. Could you please replace
DEBIAN_REV="3"
SOURCE_FILENAME_2="squashfs-tools_$portVersion-$DEBIAN_REV.debian.tar.xz"
by
debianRevision="3"
SOURCE_FILENAME_2="squashfs-tools_$portVersion-$debianRevision.debian.tar.xz"
| squashfs_tools-$portVersion-gcc2.patch" | ||
|
|
||
| ARCHITECTURES="x86_gcc2 x86 x86_64" | ||
| SECONDARY_ARCHITECTURES="x86" |
There was a problem hiding this comment.
Can please replace
SECONDARY_ARCHITECTURES="x86"
by
SECONDARY_ARCHITECTURES="x86_gcc2 x86"
(notice the blank line.)
| PROVIDES=" | ||
| squashfs_tools = $portVersion | ||
| cmd:mksquashfs = $portVersion | ||
| cmd:unsquashfs = $portVersion |
There was a problem hiding this comment.
Please change PROVIDES to:
PROVIDES="
squashfs_tools$secondaryArchSuffix = $portVersion
cmd:mksquashfs$secondaryArchSuffix = $portVersion
cmd:unsquashfs$secondaryArchSuffix = $portVersion
"
| lib:liblz4 | ||
| ${HAVE_XZ:+lib:liblzma} | ||
| lib:liblzo2 | ||
| lib:libz |
There was a problem hiding this comment.
Please change REQUIRES to:
REQUIRES="
haiku$secondaryArchSuffix
lib:liblz4$secondaryArchSuffix
${HAVE_XZ:+lib:liblzma$secondaryArchSuffix}
lib:liblzo2$secondaryArchSuffix
lib:libz$secondaryArchSuffix
"
| devel:liblz4 | ||
| ${HAVE_XZ:+devel:liblzma} | ||
| devel:liblzo2 | ||
| devel:libz |
There was a problem hiding this comment.
Please change BUILD_REQUIRES to:
BUILD_REQUIRES="
haiku${secondaryArchSuffix}_devel
devel:liblz4$secondaryArchSuffix
${HAVE_XZ:+devel:liblzma$secondaryArchSuffix}
devel:liblzo2$secondaryArchSuffix
devel:libz$secondaryArchSuffix
"
| " | ||
| BUILD_PREREQUIRES=" | ||
| cmd:gcc | ||
| cmd:make |
There was a problem hiding this comment.
Please change BUILD_PREREQUIRES to:
BUILD_PREREQUIRES="
cmd:gcc$secondaryArchSuffix
cmd:make
"
| CHECKSUM_SHA256_2="1c296cc147e322e7124bf23a3c242ac83f6a986e6d6f64829ad2424b33914fc4" | ||
|
|
||
| COMMIT="dceb729f0369d72f1d7820705fd12510b71446d2" | ||
| PATCHES_BASE_URI="https://gitweb.gentoo.org/repo/gentoo.git/plain/sys-fs/squashfs-tools/files" |
There was a problem hiding this comment.
Can you please replace
COMMIT="dceb729f0369d72f1d7820705fd12510b71446d2"
PATCHES_BASE_URI="https://gitweb.gentoo.org/repo/gentoo.git/plain/sys-fs/squashfs-tools/files"
by
commit="dceb729f0369d72f1d7820705fd12510b71446d2"
patchesBaseUri="https://gitweb.gentoo.org/repo/gentoo.git/plain/sys-fs/squashfs-tools/files"
and change the SOURCE_URI_{3-6} to:
SOURCE_URI_3="$patchesBaseUri/$SOURCE_FILENAME_3?id=$commit#noarchive"
SOURCE_URI_4="$patchesBaseUri/$SOURCE_FILENAME_4?id=$commit#noarchive"
SOURCE_URI_5="$patchesBaseUri/$SOURCE_FILENAME_5?id=$commit#noarchive"
SOURCE_URI_6="$patchesBaseUri/$SOURCE_FILENAME_6?id=$commit#noarchive"
Thanks!
BTW, here are the reasons for these cosmetic changes:
- We currenly have "COMMIT" for 4 recipes, "commit" for 22 recipes and "srcGitRev" for 42 recipes.
When I started recommending "srcGitRev" sometime ago, I did so because it was already the most used custom variable, but I had not noticed that there were almost 20 "commit"s.
Since "srcGitRev" is a bit strange, I think we should stop using it and start using "commit" instead.
As said earlier, I think we should avoid all-caps in custom variables, so I would recommend to use commit instead of COMMIT.
- haikuporter complains about PATCHES_BASE_URI and says it will ignore it.
So I would recommend to use patchesBaseUri instead of PATCHES_BASE_URI.
@waddlesplash, I'm almost sure we can find an order (to apply the patches) that will make the offsets work fine. I'll dig this right now ;-) |
@hermord, There is necessarily an order which minimizes the number of offset mismatches. I'm trying to find that order. BTW, +1 for having removed the sysmacros and xattrs patches ;-) |
Oh, definitely, it's just that EDIT: It's actually |
|
The ebuild Gentoo wrote for release 4.3 applies all the patches from Debian and then their own patches in this order:
So I guess that if we also apply the exact same patches then should have no offset mismatches when not applying our own patches (i.e. the ones you wrote), and very few if we also apply them (because haikuporter applis our patches before calling the PATCH() function if I'm not mistaken. |
|
Hmm, maybe we could, some day, teach haikuporter to:
|
|
My own patches actually don't change the offsets, expect the second |
|
Even when I manually apply the Debian patches one by one onto the original source tree, |
|
I looked into Gentoo's |
That's great. It makes the problem a little less complex ;-) Hmm, before you switch to another recipe (if you chose to do so), could you please address the comments in my last review, to add support for building for secondary architectures?
No regrets, then! Let's also ignore the offset mismatches! |
| *start_block = fragment_entry->start_block; | ||
| *size = fragment_entry->size; | ||
| + | ||
| + TRACE("read_fragment: reading fragment %d\n", fragment); |
There was a problem hiding this comment.
I think it would be better to insert a block, instead of moving TRACE down.
What about having this in the patch:
void read_fragment_3(unsigned int fragment, long long *start_block, int *size)
{
TRACE("read_fragment: reading fragment %d\n", fragment);
-
+{
squashfs_fragment_entry_3 *fragment_entry = &fragment_table[fragment];
*start_block = fragment_entry->start_block;
*size = fragment_entry->size;
-}
+}}There was a problem hiding this comment.
That shouldn't make a difference, since fragment isn't modified in the function. But if that looks cleaner, then sure.
There was a problem hiding this comment.
You are right, it does not make any difference because we are not building with TRACE enabled.
So yes, that's just to make things easier to change, should anyone wish to compile with TRACE enabled.
There was a problem hiding this comment.
I've just generated a patchset with haikuporter. It starts with a mail-style header and then contains the patch:
diff --git a/unsquash-2.c b/unsquash-2.c
index 0f2bfe8..5a3c39b 100644
--- a/unsquash-2.c
+++ b/unsquash-2.c
@@ -118,13 +118,13 @@ int read_fragment_table_2(long long *directory_table_end)
void read_fragment_2(unsigned int fragment, long long *start_block, int *size)
{
TRACE("read_fragment: reading fragment %d\n", fragment);
-
+ {
squashfs_fragment_entry_2 *fragment_entry = &fragment_table[fragment];
*start_block = fragment_entry->start_block;
*size = fragment_entry->size;
+ }
}
-
struct inode *read_inode_2(unsigned int start_block, unsigned int offset)
{
static union squashfs_inode_header_2 header;
diff --git a/unsquash-3.c b/unsquash-3.c
index d5af57a..b5177c0 100644
--- a/unsquash-3.c
+++ b/unsquash-3.c
@@ -105,13 +105,13 @@ int read_fragment_table_3(long long *directory_table_end)
void read_fragment_3(unsigned int fragment, long long *start_block, int *size)
{
TRACE("read_fragment: reading fragment %d\n", fragment);
-
+ {
squashfs_fragment_entry_3 *fragment_entry = &fragment_table[fragment];
*start_block = fragment_entry->start_block;
*size = fragment_entry->size;
+ }
}
-
struct inode *read_inode_3(unsigned int start_block, unsigned int offset)
{
static union squashfs_inode_header_3 header;In the beginning it's a bit hard to understand what we need to do to have haikuporter generate pathsets.
But basically, if you start from scratch without any patch, you:
hp -b squashfs_tools
cd work-4.3/sources/squashfs4.3
vi unsquash-2.c unsquash-3.c
git add unsquash-2.c unsquash-3.c
git commit -m "gcc2 patch" unsquash-2.c unsquash-3.c
cd ../../..
hp -e squashfs_tools
(hp is an alias to "haikuporter --get-dependencies")
Warning: "hp -e" will delete any file called squashfs_tools-4.3.patch and overwrite squashfs_tools-4.3.patchset without any notice, so don't forget to make backups ;-)
Now if you already have a patchset with several distinct patches, you can change any of them by adding a commit and then doing an interactive squash/rebase against the git revision that corresponds to the import.
I can detail that if you need.
The other difficult topic is when you have a patch for an old release, and it does not apply to the new release. It took me a while to become confortable with how to use "git add" and "git am --continue" to refresh a patchset. If you need more info I can explain what I have learned ;-)
Using patchsets is recommended, especially if the patch is not trivial, because it simplifies the maintainance of the patchsets when we upgrade a recipe.
|
I seem to be missing something, because |
|
You don't need to call setarch. |
|
|
|
|
||
| PATCHES=" | ||
| squashfs_tools-$portVersion-haiku-compat.patch | ||
| squashfs_tools-$portVersion-gcc2.patch" |
There was a problem hiding this comment.
Should be:
PATCHES="
squashfs_tools-$portVersion-haiku-compat.patch
squashfs_tools-$portVersion-gcc2.patch
"
(the closing double quotes goes to a separate line)
|
|
||
| PROVIDES=" | ||
| squashfs_tools$secondaryArchSuffix = $portVersion | ||
| cmd:mksquashfs$secondaryArchSuffix= $portVersion |
There was a problem hiding this comment.
Missing space before = in
cmd:mksquashfs$secondaryArchSuffix = $portVersion
You just need to: (Well, the -S is not required, but I always use it.) There's something I do since a few months: I have a dropliba script which gets rid of all static libs of a hpkg. (It basically unpacks the hpkg with "package extract", looks for all lib*.a files, deletes them, and recreates the package with "package create".) So everytime I build some package, I run dropliba on its devel package and then I install both the base and the devel package. |
|
When you build lz4 for x86 secondary arch, you should obtain these hpkgs in haikuports/packages/:
You don't need to install any of them before building squashfs_tools_x86, but if you later wish to install squashfs_tools_x86 you will need to also install (at least) lz4_x86. |
|
Just in case, I updated the recipe to incorporate the cosmetic edits. |
Added a recipe for squashfs_tools, a suite of tools for creating and manipulating squashfs filesystems.