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

utsushi: 3.57.0 -> 3.59.2 #70160

Merged
merged 1 commit into from Oct 12, 2019
Merged

utsushi: 3.57.0 -> 3.59.2 #70160

merged 1 commit into from Oct 12, 2019

Conversation

wucke13
Copy link
Contributor

@wucke13 wucke13 commented Oct 1, 2019

Motivation for this change
  • adding installChecks
  • building from gitlab source now
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@symphorien
Copy link
Member

Any reason to remove the patches which make this work without imagemagick in the PATH ? See #64985 (comment) for motivation.
cc @thedavidmeister for testing.
Nice to see that you enabled the tests.

@wucke13
Copy link
Contributor Author

wucke13 commented Oct 3, 2019

@symphorien no good ones. I just assumed they were some magic for making the epson distributed thing tarball work without actually looking into them. I'm sorry. I will incorporate them again and test the build, though I prefer my way of wrapping tesseract. What do you think in that regard?
The manual states wrapping should be avoided in favor of patching in the configurePhase (https://nixos.org/nixpkgs/manual/#ssec-stdenv-functions), so the patches seem to be the best approach!
Apparently, the patches do break the instalCheck. I will investigate further.
I removed the patches, and wrapped the files in question. If somebody has one of the compatible scanners, please try this out!

Using 597c8058fec8faa766e8ddfc64cb9370f4dc5e4f I was able to scan using the utsushi sane backend, but not using the utsushi binary (both only tested via usb).

@wucke13 wucke13 force-pushed the utsushi branch 3 times, most recently from f1f7fda to 597c805 Compare October 4, 2019 19:53
@symphorien
Copy link
Member

Patching is required for imagemagick because otherwise all programes using libsane should be wrapped as well, which is not possible. So please revert to using patches.

@wucke13
Copy link
Contributor Author

wucke13 commented Oct 5, 2019

@symphorien I do not understand about what libsane are we talking? Like what breaks due to libsane being wrapped?

@symphorien
Copy link
Member

If you look at the imagemagick patch, you will see that the C code calls convert explicitly. And any program on the system can load libsane and thus call this code. Therefore any program can require convert on PATH. The only reasonable solution is to replace convert by an absolute path in the C file.

@wucke13
Copy link
Contributor Author

wucke13 commented Oct 5, 2019

@symphorien Ok, done. This should cover the get-text-orientation script too.
WIth df789f2 both utsushi and xsane work for scanning (via usb).

@thedavidmeister
Copy link
Contributor

@wucke13 ok i'll try testing then

@thedavidmeister
Copy link
Contributor

xsane fails to scan:

image

@thedavidmeister
Copy link
Contributor

similar for utsushi, the scanner runs but files aren't created

@wucke13
Copy link
Contributor Author

wucke13 commented Oct 6, 2019

@thedavidmeister please give me detailed information about the setup on which you're having trouble scanning. As stated, I was able to scan both with utsushi and and xsane, so your feedback surprises me a little.

I did it like this
configuration.nix:

{
  nixpkgs.config = {
    packageOverrides = super: let self = super.pkgs; in {
      current = (import /* insert absolute path to checkout of my branch, ending with `/nixpkgs` */ {
        config = config.nixpkgs.config;
      });
    };
  };
# ...
  hardware.sane = {
    enable = true;
    extraBackends = with pkgs; [ current.utsushi ];
  };
# ...
  environment.systemPackages = with pkgs; [ xsane ];
}

The scanner that I was using is the integrated scanner of an Epson ET-3750.
So, how what exactly did you do what ultimately failed?

Edit:
Using 69b5239 I'm still able to scan both using utsushi and xsane. Please note: I have to disconnect the USB cable when switching from one software to the other, otherwise the scanner is not recognized by the other software. Furthermore, after the rebuild, a logout is necessary for sane to work, as sane uses Environment variables to specify which configuration to use, and those happen to be only reloaded on login.

@wucke13
Copy link
Contributor Author

wucke13 commented Oct 11, 2019

ping @thedavidmeister

Notice: It is somewhat pervert having something automatically merge broken = true commits on a package causing a merge conflict for a PR on that very same package where the PR potentially solves the brokenness.

+ building from gitlab source
+ reorderd dependencies to be in logical/alphabetical order
+ replaced patches with substituteInPlace, as patches broke the installChecks
  and substituteInPlace may be more robust than making new patches for each
  release. Also the compiled source is now closer to upstream than
  before.
+ added installChecks
+ added all supported devices as listed on epson.net
@grahamc
Copy link
Member

grahamc commented Oct 11, 2019

@GrahamcOfBorg build utsushi

@grahamc
Copy link
Member

grahamc commented Oct 11, 2019

(note it wasn't a bot, but #70645 and #68361)

@grahamc
Copy link
Member

grahamc commented Oct 11, 2019

@GrahamcOfBorg eval

@wucke13
Copy link
Contributor Author

wucke13 commented Oct 11, 2019

@grahamc Please backport too, I would like to not wait another 5 1/2 months until I can scan with my scanner on a stable system... Also: thank you for taking the time on this matter!

@grahamc grahamc merged commit 9c19b7b into NixOS:master Oct 12, 2019
@grahamc
Copy link
Member

grahamc commented Oct 12, 2019

@wucke13 Thank you! Please open another PR backporting ot 19.09, with git cherry-pick -x 9eb00acbbee63d20a6ac7c124005089a8bebcfba.

@wucke13 wucke13 mentioned this pull request Oct 12, 2019
10 tasks
@thedavidmeister
Copy link
Contributor

my testing was pretty simple, i set my nixos channel to the fork commit and ran nixos-rebuild

i have all the packages included as explained above, i was using utsushi when it was first merged

as this is merged i'll swap back to the latest master, rebuild nixos and see what happens 🤷‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants