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

New package: sane-airscan-0.99.24 #29627

Merged
merged 1 commit into from Mar 26, 2021
Merged

Conversation

pudiva
Copy link
Contributor

@pudiva pudiva commented Mar 20, 2021

General

Have the results of the proposed changes been tested?

  • I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me
  • I generally don't use the affected packages but briefly tested this PR

@ericonr ericonr added the new-package This PR adds a new package label Mar 20, 2021
@ericonr
Copy link
Member

ericonr commented Mar 20, 2021

Duplicate of #29392, but I haven't looked at the difference between the templates yet.

@oynqr
Copy link
Contributor

oynqr commented Mar 22, 2021

#29392 uses meson and this uses make.
It seems like both are supported by upstream and other distros are not largely favoring one either, with Arch and Fedora using make and Nix and ALT using meson.

@pudiva
Copy link
Contributor Author

pudiva commented Mar 22, 2021

In case people prefer to go with my version (make) rather than #29392 (meson), I just added the line to shlibs that I had forgotten...

...my template had to define the do_install() because the gnu-makefile style defines the variable STRIP=true, which breaks this particular pkg. I usually favour make over other build systems because it's a standard tool ~ which means less dependencies ~ but since void already suports meson, then perhaps it's better to go with it, as it requires less template code to maintain.

Comment on lines 17 to 18
: ${make_cmd:=make}
: ${make_install_target:=install}
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 okay to hardcode these in the command. What you might be able to do is add make_install_args="STRIP=echo" or use some conditionals to get the correct strip for cross and native compilation there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ericonr, thanks for the suggestion! Indeed I could get around defining a custom do_install() by setting $make_install_args="STRIP=-s"...

However, Makefile uses $(STRIP) as options to install:
https://github.com/alexpevzner/sane-airscan/blob/master/Makefile#L118-L132

and defaults to -s (which instructs install to strip):
https://github.com/alexpevzner/sane-airscan/blob/master/Makefile#L30-L35

so, for cross-compilation, I think we would need to override $(INSTALL) with target-linux-gnu-install ~ which could be a nice addition to the gnu-makefile build style, as it already does many similar overrides for CC, LD and friends.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually install --strip-program=target-linux-gnu-strip. Would be a bit strange to have platform-specific install...

Copy link
Member

Choose a reason for hiding this comment

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

We can do STRIP=-s, because we have an install wrapper that removes the flag anyway :p So you don't need to care about passing the correct --strip-program :)

We do the stripping ourselves, to generate debug packages. I quite dislike build systems that try to strip binaries themselves, tbh.

@ericonr
Copy link
Member

ericonr commented Mar 22, 2021

Maybe report it upstream, but the makefile version is missing the unversioned symlink:

/usr/lib/sane/libsane-airscan.so -> /usr/lib/sane/libsane-airscan.so.1

@pudiva
Copy link
Contributor Author

pudiva commented Mar 25, 2021

Maybe report it upstream, but the makefile version is missing the unversioned symlink:

/usr/lib/sane/libsane-airscan.so -> /usr/lib/sane/libsane-airscan.so.1

I submitted a PR, but upstream didn't like the unversioned symlink, and also shared that they don't really maintain the meson build, but just try to not break it ~ sounds like they prefer make. Their full response:

Nobody links against these drivers directly, they are always loaded via sane-dll, which doesn't use unversioned symlink. So I believe that SANE itself creates these links only for legacy reasons and there is no need to reproduce this behaviour in the independent package.

Official packages for Fedora, Debian and Ubuntu come without this symlink and it causes no problems and no complains.

Regarding meson build, it was contributed by my users and I don't maintain it, I only keep it here for everybody's convenience and trying not to break when something changes in the project.

So really thank for your contribution, but in this particular case I don't think we really need it.

alexpevzner/sane-airscan#132 (comment)

@ericonr
Copy link
Member

ericonr commented Mar 25, 2021

In that case, I think we should go with the Makefile based build.

common/shlibs Outdated Show resolved Hide resolved
make_install_args="STRIP="
hostmakedepends="pkg-config"
makedepends="avahi-libs-devel sane-devel gnutls-devel libxml2-devel libjpeg-turbo-devel libpng-devel"
short_desc="SANE driver for Apple AirScan"
Copy link
Member

Choose a reason for hiding this comment

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

Could you make the description closer to what's used by upstream?

Scanner Access Now Easy - universal driver for eSCL (Apple AirScan) and WSD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ericonr ericonr mentioned this pull request Mar 26, 2021
3 tasks
@ericonr
Copy link
Member

ericonr commented Mar 26, 2021

Wooh!

@ericonr ericonr merged commit 3f9f719 into void-linux:master Mar 26, 2021
@pudiva pudiva deleted the sane-airscan branch March 26, 2021 03:17
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new-package This PR adds a new package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants