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

cutter: update to 2.0.5. #35901

Merged
merged 3 commits into from
Mar 4, 2022
Merged

cutter: update to 2.0.5. #35901

merged 3 commits into from
Mar 4, 2022

Conversation

faulesocke
Copy link
Contributor

Testing the changes

  • I tested the changes in this PR: YES

Local build testing

  • I built this PR locally for my native architecture, (x86_64-glibc)

@faulesocke faulesocke changed the title cutter: update to 2.0.5. WIP: cutter: update to 2.0.5. Mar 1, 2022
@faulesocke faulesocke force-pushed the cutter branch 2 times, most recently from 6246679 to c109e2e Compare March 2, 2022 08:55
@faulesocke faulesocke changed the title WIP: cutter: update to 2.0.5. cutter: update to 2.0.5. Mar 2, 2022
qt5-location-devel qt5-svg-devel radare2"
version=2.0.5
revision=1
archs="i686 x86_64"
Copy link
Member

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does not build for arm.

Copy link
Member

Choose a reason for hiding this comment

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

Is this documented upstream? What's the error? Does it build for -musl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't checked the error and haven't checked musl, this is just what I've seen from a previous pipeline run. I don't have time for fiddling around with these architectures. When someone needs it on musl or arm he can still make a PR to fix it.

Copy link
Member

Choose a reason for hiding this comment

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

It's not acceptable to just disable a package for some architectures without a good reason. Remove these arch restrictions from both packages so we can see where it fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tree-sitter has nocross=yes. So it won't compile to arm.

Copy link
Member

Choose a reason for hiding this comment

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

But tree-sitter can still be compiled natively on arm. In that case we should set nocross=yes instead of restricting archs, but #35922 should fix cross compilation of tree-sitter, so this line can be dropped.

Comment on lines 23 to 30
do_fetch() {
if [ ! -e "$wrksrc" ]; then
git clone --recurse-submodules "https://github.com/rizinorg/${pkgname}" $wrksrc
fi
cd $wrksrc
git checkout "v${version}"
git submodule update --init --recursive
}
Copy link
Member

Choose a reason for hiding this comment

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

This is very ugly, can we just use release tarballs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They don't offer release source tarballs. I've several other packages in my pipeline with the same issue. Looks like package maintainers don't know (or don't care) that github does not include submodules into automatically generated tarballs. Probably because of things like flatpack. But other distributions don't seem to care as well: They just fetch the source from github directly, like I did.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an issue for this which is unanswered since ~2 months: rizinorg/cutter#2878

I guess they have more important stuff to do and we have to go with the "ugly" solution.

Copy link
Member

Choose a reason for hiding this comment

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

That's unfortunate, but could you do something similar to #32199?

@faulesocke
Copy link
Contributor Author

What can I do to skip the do_check in the pipeline?

distfiles="https://github.com/rizinorg/${pkgname}/releases/download/v${version}/${pkgname}-src-v${version}.tar.xz"
checksum=eea49b396387c09d19705aab02a617cdb15682fca67f101ff2b27eef94a710e9

subpackages="rizin-devel"
Copy link
Member

Choose a reason for hiding this comment

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

this is not required



rizin-devel_package() {
depends="${makedepends} rizin>=${version}_${revision}"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
depends="${makedepends} rizin>=${version}_${revision}"
depends="${makedepends} ${sourcepkg}>=${version}_${revision}"

Comment on lines 28 to 30
pkg_install() {
vmove usr/include/librz
vmove usr/lib/pkgconfig
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pkg_install() {
vmove usr/include/librz
vmove usr/lib/pkgconfig
}
pkg_install() {
vmove "usr/lib/*.so"
vmove usr/include/librz
vmove usr/lib/pkgconfig
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The .so files definitely DO NOT go into the -devel package.

Copy link
Member

Choose a reason for hiding this comment

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

They are just symlinks to .so.$version, we don't need them in the main package, we only need the real versioned libraries.

@faulesocke faulesocke force-pushed the cutter branch 2 times, most recently from 2d8ae56 to d27c1eb Compare March 2, 2022 12:08
distfiles="https://github.com/radareorg/${pkgname}/archive/v${version}.tar.gz"
checksum=868213d2ea0b4a29b9b03c9b605c2b2155c4c03b62735a9bd376a0dadcb4fe1b
homepage="https://cutter.re"
_translations_commit="974298653ba71b958e1b6c83f6011f5fefff6236"
Copy link
Member

Choose a reason for hiding this comment

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

put this next to $version

qt5-location-devel qt5-svg-devel radare2"
version=2.0.5
revision=1
archs="i686 x86_64"
Copy link
Member

Choose a reason for hiding this comment

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

But tree-sitter can still be compiled natively on arm. In that case we should set nocross=yes instead of restricting archs, but #35922 should fix cross compilation of tree-sitter, so this line can be dropped.

@Johnnynator Johnnynator merged commit 9f16af8 into void-linux:master Mar 4, 2022
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