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

remarkable-mouse: init at 5.2.1 #85899

Merged
merged 5 commits into from Jun 14, 2020
Merged

remarkable-mouse: init at 5.2.1 #85899

merged 5 commits into from Jun 14, 2020

Conversation

@NickHu
Copy link
Contributor

NickHu commented Apr 23, 2020

Motivation for this change

Add remarkable-mouse, a
tool to use the reMarkable as a graphics tablet, and its dependencies.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.
@NickHu NickHu requested review from FRidh and jonringer as code owners Apr 23, 2020
@flokli flokli changed the title remarkable-mouse: init at 5.1 pythonPackages.libevdev: init at 0.7 pythonPackages.setuptools-lint: init at 0.6.0 pythonPackages.pynput: init at 1.6.8 pythonPackages.screeninfo: init at 0.6.5 remarkable-mouse: init Apr 24, 2020
Copy link
Contributor

flokli left a comment

Thanks for packaging this. Some comments - also, you might want to restrict this to py3k only.

substituteInPlace screeninfo/enumerators/xinerama.py \
--replace "load_library(\"X11\")" "ctypes.cdll.LoadLibrary(\"${libX11}/lib/libX11.so\")" \
--replace "load_library(\"Xinerama\")" "ctypes.cdll.LoadLibrary(\"${libXinerama}/lib/libXinerama.so\")"
substituteInPlace screeninfo/enumerators/xrandr.py \
--replace "load_library(\"X11\")" "ctypes.cdll.LoadLibrary(\"${libX11}/lib/libX11.so\")" \
--replace "load_library(\"Xrandr\")" "ctypes.cdll.LoadLibrary(\"${libXrandr}/lib/libXrandr.so\")"
'';
Comment on lines +17 to +23

This comment has been minimized.

Copy link
@flokli

flokli Apr 24, 2020

Contributor

load_library is defined in utils.py, and is basically just a wrapper around

path = ctypes.util.find_library(name)
return ctypes.cdll.LoadLibrary(path)

On Linux, this uses dlopen, so if you just wrapProgram the binaries to add libX11 and libXrandr's lib path to LD_LIBRARY_PATH, these libraries should be found even without patching the source.

This comment has been minimized.

Copy link
@jtojnar

jtojnar Apr 24, 2020

Contributor

I do not like using LD_LIBRARY_PATH since it can lead to weird and difficult to diagnose error when it loads a different version of library than the program is compiled against. Remember environment variables are inherited by child processes and LD_LIBRARY_PATH has higher priority than DT_RUNPATH ELF entry we use to locate libraries.

For a concrete example when this occurs: If you have a URL hyperlink in the application and click it, it will open a browser child process, which will inherit these X libraries. If you use browser from a different channel, it will likely crash.

This comment has been minimized.

Copy link
@flokli

flokli Apr 24, 2020

Contributor

Fair enough - Can we still add the two library paths into utils.py, instead of hunting for each and every occurence?

This comment has been minimized.

Copy link
@NickHu

NickHu May 17, 2020

Author Contributor

I could patch the load_library function in utils.py, but it felt neater to me to just use substituteInPlace as those libraries are highly unlikely to appear anywhere except in the files which get patched

sha256 = "10gwj08kn2rs4waq7807mq34cbavgkpg8fpir8mvnba601b8q4r4";
};

doCheck = false;

This comment has been minimized.

Copy link
@flokli

flokli Apr 24, 2020

Contributor

Why were the tests disabled?

This comment has been minimized.

Copy link
@NickHu

NickHu Apr 24, 2020

Author Contributor

They crash because they can't find libraries.

This comment has been minimized.

Copy link
@flokli

flokli Apr 24, 2020

Contributor

Can you add a small comment with an example? Maybe s.o can find time to debug it.
It's preferrable to run the tests if possible, so breakages caused by other python package bumps are more likely to be noticed.

This comment has been minimized.

Copy link
@jonringer

jonringer Apr 28, 2020

Contributor

crashing isn't a good sign, could mean that it's broken in a sandbox, thus nixpkgs

sha256 = "10gwj08kn2rs4waq7807mq34cbavgkpg8fpir8mvnba601b8q4r4";
};

doCheck = false;

This comment has been minimized.

Copy link
@jonringer

jonringer Apr 28, 2020

Contributor

crashing isn't a good sign, could mean that it's broken in a sandbox, thus nixpkgs

pkgs/development/python-modules/pynput/default.nix Outdated Show resolved Hide resolved
@jonringer
Copy link
Contributor

jonringer commented May 2, 2020

Please follow CONTRIBUTING.md and manual#submitting-changes-making-patches and squash the fixup commits.

NickHu added 4 commits Apr 23, 2020
@NickHu NickHu force-pushed the NickHu:remarkable-mouse branch from 6170c12 to e4da232 May 17, 2020
@NickHu
Copy link
Contributor Author

NickHu commented May 17, 2020

Squashed and rebased @jonringer
I'm more or less happy with this now, is there anything else blocking merge?

@NickHu NickHu force-pushed the NickHu:remarkable-mouse branch from e4da232 to a3c6bf8 May 17, 2020
@NickHu NickHu changed the title remarkable-mouse: init remarkable-mouse: init at 5.2.1 May 17, 2020
@NickHu
Copy link
Contributor Author

NickHu commented May 17, 2020

@GrahamcOfBorg build remarkable-mouse

@flokli flokli requested a review from lheckemann May 18, 2020
@lheckemann
Copy link
Member

lheckemann commented Jun 14, 2020

Sorry I took so long to get around to this! Seems to work, will fix the conflicts and merge.

@lheckemann lheckemann closed this Jun 14, 2020
@lheckemann lheckemann merged commit 2c6fcb0 into NixOS:master Jun 14, 2020
16 of 18 checks passed
16 of 18 checks passed
remarkable-mouse on aarch64-linux
Details
remarkable-mouse, remarkable-mouse.passthru.tests on aarch64-linux
Details
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a3c6bf8"; rev="a3c6bf8c951476db29610948c6e6099213795643"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a3c6bf8"; rev="a3c6bf8c951476db29610948c6e6099213795643"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a3c6bf8"; rev="a3c6bf8c951476db29610948c6e6099213795643"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a3c6bf8"; rev="a3c6bf8c951476db29610948c6e6099213795643"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a3c6bf8"; rev="a3c6bf8c951476db29610948c6e6099213795643"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a3c6bf8"; rev="a3c6bf8c951476db29610948c6e6099213795643"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a3c6bf8"; rev="a3c6bf8c951476db29610948c6e6099213795643"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
remarkable-mouse on x86_64-linux Success
Details
remarkable-mouse, remarkable-mouse.passthru.tests on x86_64-linux Success
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.