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

keybase, keybase-gui, kbfs: 4.6.0 -> 4.7.2, added dependencies #71287

Merged
merged 1 commit into from Nov 3, 2019
Merged

keybase, keybase-gui, kbfs: 4.6.0 -> 4.7.2, added dependencies #71287

merged 1 commit into from Nov 3, 2019

Conversation

judaew
Copy link
Contributor

@judaew judaew commented Oct 17, 2019

Motivation for this change

Adding:

  • fuse or osxfuse as a dependency for kbfs (I rely on this kbfs README.md#status)
  • gnupg as a dependency for keybase (for command $ keybase pgp gen)
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 @

@worldofpeace
Copy link
Contributor

worldofpeace commented Oct 19, 2019

@judaew Are these used during the build, or are the executable/s needed in PATH during runtime?
If so we need to hardcode the paths in the source code.

@judaew
Copy link
Contributor Author

judaew commented Oct 19, 2019

@worldofpeace Yes, executables are needed at runtime. I analyzed the code and I came to the conclusion that Keybase already has GnuPG support for Nix (pinentry_nix.go).

I was a little skeptical about fuse. But the experiment showed that PATH is found automatically. I previously removed fuse from Ubuntu and verified that kbfs is not working properly because of this.

But I'm not sure about osxfuse, since it looks like the location is set within lines options.go#L295-L313. And within lines 285 to 290 the path to the native osxfuse, if another is not available. I think it should work both ways.

@judaew judaew changed the title keybase: adding dependencies fuse, osxfuse, gnupg keybase: 4.6.0 -> 4.7.1 and adding dependencies fuse, osxfuse, gnupg Oct 23, 2019
@worldofpeace
Copy link
Contributor

Well what I meant by this was, if the program needs executables from fuse to be present at runtime like

result/bin
├── fusermount
├── mount.fuse
└── ulockmgr_server

they won't be there even if they're added to buildInputs. They'll just be in PATH during the build.

Nix doesn't really manage things in this way, though perhaps it should/could.
So what usually has to be done is substitute the calls to any of those executables with a hardcoded path within the nix store. Here's a precise example of what I usually request people to perform

Though I believe this approach would be rather difficult if they were to switch to go modules, or not vendor things.


There also seems to be some assumptions going for macos

Now if nix was sandboxed these wouldn't work because they're on the host system, but I believe with macos it would be on the safe side to just rely on these being present outside of nix.

@judaew
Copy link
Contributor Author

judaew commented Oct 27, 2019

Something just like this?

Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

It does appear you've performed the patch correctly. Will test.

@ofborg ofborg bot requested review from ehmry, bennofs, np and puffnfresh October 31, 2019 20:54
@worldofpeace
Copy link
Contributor

@judaew If you can squash your commits accordingly I can merge.

@judaew judaew changed the title keybase: 4.6.0 -> 4.7.1 and adding dependencies fuse, osxfuse, gnupg keybase, keybase-gui, kbfs: 4.6.0 -> 4.7.2, added dependencies Nov 1, 2019
@judaew
Copy link
Contributor Author

judaew commented Nov 1, 2019

@worldofpeace Well, I did it.

@ofborg ofborg bot requested a review from ehmry November 1, 2019 23:44
all: update from 4.6.0 to 4.7.2

keybase:
- added gnupg as a dependency and patch fix-patch-keybase.patch

kbfs:
- added fuse as a dependency and patch fix-patch-kbfs.patch
@worldofpeace
Copy link
Contributor

@GrahamcOfBorg build keybase kbfs keybase-gui

@worldofpeace
Copy link
Contributor

Running checks for darwin. I had to update the keybase-gui expression because it 404'd for me.

@worldofpeace worldofpeace merged commit e43ab14 into NixOS:master Nov 3, 2019
@worldofpeace
Copy link
Contributor

Thanks for your patience @judaew and the contributions ✨

@judaew
Copy link
Contributor Author

judaew commented Nov 3, 2019

Thanks for your help @worldofpeace

@judaew judaew deleted the dep/keybase branch November 12, 2019 21:15
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

3 participants