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: 1.0.18 -> 20170209.17b641d #22600

Merged
merged 1 commit into from Feb 17, 2017
Merged

Conversation

SuprDewd
Copy link
Contributor

@SuprDewd SuprDewd commented Feb 9, 2017

Motivation for this change

keybase is out of date.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

I'm bumping kbfs's version simultaneously in #22601. Both programs seem to be actively developed over at Keybase, but it seems that they've stopped making formal releases for keybase, and there were never any formal releases for kbfs. Instead they seem to be something like a rolling release (see https://s3.amazonaws.com/prerelease.keybase.io/linux_binaries/deb/index.html). I picked the commits for keybase/kbfs based on (at the time of writing this) newest entry in that list.

This also fixes some warnings that were being generated due to a slight mismatch between the current versions (in nixpkgs) of keybase and kbfs. See #22375.

@mention-bot
Copy link

@SuprDewd, thanks for your PR! By analyzing the history of the files in this pull request, we identified @aristidb and @carlsverre to be potential reviewers.

@carlsverre
Copy link
Contributor

Can we have the version that tracks master be named separately? This seems to be a random commit from master that happened in the last 6 hours. I would rather use "official" releases from their repo since this is security software.

@SuprDewd
Copy link
Contributor Author

SuprDewd commented Feb 9, 2017

That sounds like a good idea. So something like keybase-stable for the old version? (Someone have a better name?) Or should it be the other way around, with keybase pointing to the old version, and keybase-unstable pointing to this version?

But to justify these particular commits: this is the same version as you would get if you'd go to https://keybase.io/download and download the software. So this is, to some extent, an "official" release.

In any case, I think we should go with your suggestion because of the reason you gave. If anyone wants to do that, feel free to take the lead. If not, I'll try to give it a go.

@carlsverre
Copy link
Contributor

NVM! I was going off of the latest release version on their GH. I just downloaded and verified that the version of keybase you get from https://prerelease.keybase.io/keybase_amd64.deb tracks master so I think this is a safe change. Possibly we should switch to installing from their release deb although that can be another change.

IMO I am personally ok with running w/e keybase releases on their site - so I am ok with this replacing the package called keybase in nixpkgs. If other people want to run the last "official" tagged release we can add a "keybase-stable" package.

@puffnfresh
Copy link
Contributor

I think that .deb archive now also contains their Electron desktop client. If we switch to using their .deb then we'll also get access to that.

@carlsverre
Copy link
Contributor

Yea it looks like that's the case. I would love to simplify into a single nix package based on the deb with a option to disable the fuse mount. Or if the fuse mount is just initialized by the desktop client then we can install everything and people can opt in based on how they use keybase. I personally only use the CLI client.

@SuprDewd
Copy link
Contributor Author

SuprDewd commented Feb 9, 2017 via email

@SuprDewd
Copy link
Contributor Author

I think we've reached the conclusion that we want to package the .deb. If there are no objections I'll open an issue where we can discuss the next steps. I'll leave this and #22601 open in case someone wants to merge a short-term solution (especially for those that are eager to try out the new chat feature).

@carlsverre
Copy link
Contributor

Yup patchelf is currently totally messed up on go binaries. The best workaround is probably what you did which is wrapping the keybase binaries with a script that execs the binary with the nix loader. As you pointed out this can be circumvented when a binary tries to exec itself. I am currently investigating fixing patchelf so it works with ELF's produced by go since I am running into this issue quite often.

@puffnfresh
Copy link
Contributor

Maybe we'll have to go a full FHS environment in the case we can't fix patchelf or the forking.

@puffnfresh
Copy link
Contributor

I've got the start of a FHS environment here:

https://github.com/puffnfresh/nix-files/blob/master/keybase.nix

The daemon seems to start and the GUI opens but the GUI says that it can't connect:

Keybase is currently unreachable. Trying to reconnect you...

@puffnfresh
Copy link
Contributor

Got it working once I ran keybase and kbfsfuse outside of the chroot environment. Will figure out a way to get that packaged.

@SuprDewd
Copy link
Contributor Author

@puffnfresh Cool! Keep us posted. I'll withdraw the pull requests if you manage to package it.

@carlsverre
Copy link
Contributor

@puffnfresh looking forward to seeing the chroot'ed version. Curious how KBFS is going to react to having the mount point outside of the chroot. Will we need to mount it inside the chroot for things to work?

On the patchelf front - I think I have found the bug that causes it to fail to work for go compiled ELF's - and I have an extremely hacky solution that seems to work (I successfully used it to patch the deb binary version of the keybase binary to a nix packaged ld). Going to keep pushing on that since it is widely applicable for us due to the prevalence of go binaries these days. Don't block on me though since getting an updated keybase in nixpkgs should be top priority.

@bdimcheff
Copy link
Contributor

I think it'd be nice to have both keybase/kbfs clients as a separate option from the package that includes the electron app and everything. For one, I don't think there's a way to actually build electron apps deterministically so we're stuck using the precompiled deb instead of building from source. Also if you don't want the gui it's gonna be a really huge package just to get a couple go binaries.

@puffnfresh
Copy link
Contributor

I think we should build keybase and kbfs using Go and I'll submit a PR for a "keybase-gui" attribute which uses the Debian package.

@bdimcheff
Copy link
Contributor

fwiw i tried out this and #22601 and both seem to work for me

@globin globin merged commit c1fb4ec into NixOS:master Feb 17, 2017
@SuprDewd SuprDewd deleted the update-keybase branch February 17, 2017 01:46
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

7 participants