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

gitkraken: 6.4.1 -> 6.5.1 #78420

Merged
merged 1 commit into from Feb 10, 2020
Merged

gitkraken: 6.4.1 -> 6.5.1 #78420

merged 1 commit into from Feb 10, 2020

Conversation

@evanjs
Copy link
Member

@evanjs evanjs commented Jan 24, 2020

Motivation for this change

https://support.gitkraken.com/release-notes/current/#version-651

Things done
  • use ar instead of dpkg for unpackCmd to avoid attempts by the package
    to change permissions and etc.
  • add at-spi2-core dependency
  • remove dpkg dependency

Open to comments on the change to the unpackCmd. As far as I can tell, the application seems to work the same as before.

  • 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.
@megheaiulian
Copy link
Contributor

@megheaiulian megheaiulian commented Jan 31, 2020

@evanjs If you want to drop dpkg dependency it might also make sense to fetch the *.tar.gz file instead of the *.deb one.

Copy link
Contributor

@megheaiulian megheaiulian left a comment

Tested this and the executable works correctly. If you want to go with the suggestion to fetch tar.gz it would be great. Otherwise it can be done in a future update.

@evanjs
Copy link
Member Author

@evanjs evanjs commented Jan 31, 2020

diff --git a/pkgs/applications/version-management/gitkraken/default.nix b/pkgs/applications/version-management/gitkraken/default.nix
index 83368932e25..73d376396a6 100644
--- a/pkgs/applications/version-management/gitkraken/default.nix
+++ b/pkgs/applications/version-management/gitkraken/default.nix
@@ -1,7 +1,7 @@
 { stdenv, libXcomposite, libgnome-keyring, makeWrapper, udev, curl, alsaLib
 , libXfixes, atk, gtk3, libXrender, pango, gnome3, cairo, freetype, fontconfig
 , libX11, libXi, libxcb, libXext, libXcursor, glib, libXScrnSaver, libxkbfile, libXtst
-, nss, nspr, cups, fetchurl, expat, gdk-pixbuf, libXdamage, libXrandr, dbus
+, nss, nspr, cups, fetchzip, expat, gdk-pixbuf, libXdamage, libXrandr, dbus
 , makeDesktopItem, openssl, wrapGAppsHook, at-spi2-atk, at-spi2-core, libuuid
 , e2fsprogs, krb5
 }:
@@ -15,11 +15,14 @@ stdenv.mkDerivation rec {
   pname = "gitkraken";
   version = "6.5.1";
 
-  src = fetchurl {
-    url = "https://release.axocdn.com/linux/GitKraken-v${version}.deb";
-    sha256 = "1zcbbszz7vnwgd4hxwhbp28gminc3pqk0v7id7jc8ylnbgpiy2bk";
+  src = fetchzip {
+    url = "https://release.axocdn.com/linux/GitKraken-v${version}.tar.gz";
+    sha256 = "0dwjwismv4rfw58801g2ay51h9qrffcxgbl910frd4i530w0y44p";
   };
 
+  dontBuild = true;
+  dontConfigure = true;
+
   libPath = makeLibraryPath [
     stdenv.cc.cc.lib
     curlWithGnuTls
@@ -73,29 +76,24 @@ stdenv.mkDerivation rec {
   nativeBuildInputs = [ makeWrapper wrapGAppsHook ];
   buildInputs = [ gtk3 gnome3.adwaita-icon-theme ];
 
-  unpackCmd = ''
-    mkdir out
-    pushd out
-
-    # manually unpack deb to avoid attempts at changing utime, permissions, etc
-    ar x $curSrc
-    tar -xf data.tar.xz
-    popd
-  '';
-
   installPhase = ''
     runHook preInstall
-    mkdir $out
-    pushd usr
-    pushd share
-    substituteInPlace applications/gitkraken.desktop \
-      --replace /usr/share/gitkraken $out/bin
-    popd
-    rm -rf bin/gitkraken share/lintian
-    cp -av share bin $out/
-    popd
 
+    mkdir -p $out/share/gitkraken/
+    cp -R $src/* $out/share/gitkraken/
+
+    mkdir -p $out/bin
     ln -s $out/share/gitkraken/gitkraken $out/bin/gitkraken
+
+    mkdir -p $out/share/applications
+    cp ${desktopItem}/share/applications/* $out/share/applications/
+
+    substituteInPlace $out/share/applications/gitkraken.desktop \
+      --replace $out/usr/share/gitkraken $out/bin
+
+    mkdir -p $out/share/pixmaps
+    cp gitkraken.png $out/share/pixmaps/gitkraken.png
+
     runHook postInstall
   '';
 

Mrm this seems to work, just not sure on the cleanliness

@megheaiulian
Copy link
Contributor

@megheaiulian megheaiulian commented Jan 31, 2020

That looks good to me and is in fact cleaner that what's being done for the deb version. I think you should commit that.

- use fetchzip to retrieve the tarball for GitKraken, as the deb now tries to change permissions and etc which nix will not like
- add at-spi2-core dependency
- remove dpkg dependency
- refactor expression to properly handle the GitKraken tarball (compared to the deb archive)
@evanjs evanjs force-pushed the evanjs:gitkraken-6.5.0 branch from 0cdfe68 to 7c9bd98 Jan 31, 2020
@evanjs
Copy link
Member Author

@evanjs evanjs commented Jan 31, 2020

That looks good to me and is in fact cleaner that what's being done for the deb version. I think you should commit that.

Sounds good. Thanks for the review!

@evanjs evanjs changed the title gitkraken: 6.4.1 -> 6.5.0 gitkraken: 6.4.1 -> 6.5.1 Feb 3, 2020
@evanjs evanjs requested a review from jonringer Feb 7, 2020
@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Feb 10, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/need-help-upgrade-a-package-gitkraken/5829/2

@worldofpeace worldofpeace merged commit 9c22a82 into NixOS:master Feb 10, 2020
15 checks passed
15 checks passed
gitkraken on aarch64-linux No attempt
Details
gitkraken on x86_64-linux No attempt
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="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
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
@evanjs evanjs removed the request for review from jonringer Feb 10, 2020
@evanjs evanjs deleted the evanjs:gitkraken-6.5.0 branch Feb 10, 2020
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

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