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.

Sign up for GitHub

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

Jump to bottom

arc-theme: 2017-05-12 -> 2018-07-13 #43490

Merged

xeji merged 1 commit into NixOS:master from rembo10:update-arc-theme

Sep 26, 2018

Contributor

rembo10 commented Jul 13, 2018 •

edited

Motivation for this change

The horst3180 repo was unmaintained - this is the updated version as per this comment.

Updated build dependencies. Plank theme is installed automatically now, and there is no chrome theme

Still learning nix so criticism is welcome!

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 nox --run "nox-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)
Fits CONTRIBUTING.md.

GrahamcOfBorg added 8.has: clean-up 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels

infinisil reviewed

View reviewed changes

pkgs/misc/themes/arc/default.nix

    
                buildInputs = [ gnome3.gtk ];

                preBuild = ''

                  # Shut up inkscape's warnings

                  export HOME="$NIX_BUILD_ROOT"

Member

infinisil Jul 15, 2018

What warning?

infinisil reviewed

View reviewed changes

pkgs/misc/themes/arc/default.nix

    
                preferLocalBuild = true;

                propagatedUserEnvPkgs = [ gnome-themes-standard gtk-engine-murrine ];

Member

infinisil Jul 15, 2018

I just learned about propagatedUserEnvPkgs today, but there seems to be a movement to deprecate it: #43049. Although currently this is how all packages use gtk-engine-murrine, and it seems like it's needed for those, because the result isn't a binary you can just wrap.

Contributor Author

rembo10 commented Jul 15, 2018 via email •

edited

I can't remember the exact message but the gist was that inkscape couldn't create it's config or log directory, and it was spitting out that warning every time render-assets.sh was called. I can put this in a wrapper. I will have a look

romildo reviewed

View reviewed changes

pkgs/misc/themes/arc/default.nix Outdated

    
                # treat versions newer than 3.22 as 3.22

                gnomeVersion = if stdenv.lib.versionOlder "3.22" gnome3.version then "3.22" else gnome3.version;

                # per README: For GNOME 3.24 and 3.26, use --with-gnome=3.22

                gnomeVersion = if gnome3.version == "3.24" || gnome3.version == "3.26" then "3.22" else gnome3.version;

Contributor

romildo Sep 25, 2018

Maybe it is better to rely on the automatic detection of the gnome version provided by the package itself. Reading its code reveals it is in fact based on the gtk+ version, obtained with the help of pkg-config (pkg-config --modversion gtk+-3.0). It seems that in future release this will be changed.

So I suggest to remove those explicit handling of the gnome version.

romildo reviewed

View reviewed changes

pkgs/misc/themes/arc/default.nix Outdated

    
                pname = "arc-theme";

              in

              stdenv.mkDerivation rec {

                name = "${pname}-${version}";

                version = "2017-05-12";

                version = "2018-07-13";

Contributor

romildo Sep 25, 2018

It may be better to use a released version, instead of a simple revision. The latest released version is 20180715, only two days after the revision used in the PR.

romildo reviewed

View reviewed changes

pkgs/misc/themes/arc/default.nix Outdated

    
                  install -Dm644 -t $out/share/doc/${pname}        AUTHORS *.md

                '';

                meta = with stdenv.lib; {

                  description = "A flat theme with transparent elements for GTK 3, GTK 2 and Gnome-Shell";

                  homepage    = https://github.com/horst3180/arc-theme;

                  description = "[New upstream] A flat theme with transparent elements";

Contributor

romildo Sep 25, 2018

Maybe it is better not to change the description. [New upstream] may not be clear enough to convey the fact that we are using a fork of the now unmaintained package.


          arc-theme: 2017-05-12 -> 20180715

380d404

rembo10 force-pushed the update-arc-theme branch from 9493ca7 to 380d404 Compare

September 26, 2018 09:14

Contributor Author

rembo10 commented Sep 26, 2018

@romildo I agree with all your comments and made those changes.

Contributor

romildo commented Sep 26, 2018

@xeji Maybe you can take a look at this PR and merge it into master. It is becoming old.

Contributor

xeji commented Sep 26, 2018

@GrahamcOfBorg build arc-theme

GrahamcOfBorg commented Sep 26, 2018

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: arc-theme

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

GrahamcOfBorg commented Sep 26, 2018

Success on x86_64-linux (full log)

Attempted: arc-theme

Partial log (click to expand)

make[2]: Nothing to be done for 'install-data-am'.
make[2]: Leaving directory '/build/source'
make[1]: Leaving directory '/build/source'
glibPreFixupPhase
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/5485lcpgc18si0hgrhq5wq3z4hgzbj6n-arc-theme-20180715
strip is /nix/store/h0lbngpv6ln56hjj59i6l77vxq25flbz-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/5485lcpgc18si0hgrhq5wq3z4hgzbj6n-arc-theme-20180715
checking for references to /build in /nix/store/5485lcpgc18si0hgrhq5wq3z4hgzbj6n-arc-theme-20180715...
/nix/store/5485lcpgc18si0hgrhq5wq3z4hgzbj6n-arc-theme-20180715

GrahamcOfBorg commented Sep 26, 2018

Success on aarch64-linux (full log)

Attempted: arc-theme

Partial log (click to expand)

make[2]: Nothing to be done for 'install-data-am'.
make[2]: Leaving directory '/build/source'
make[1]: Leaving directory '/build/source'
glibPreFixupPhase
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/b8svz8gnl07hvnppch6xd188mypnc00w-arc-theme-20180715
strip is /nix/store/y4ymnvgxygpq05h03kyzbj572zmh6zla-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/b8svz8gnl07hvnppch6xd188mypnc00w-arc-theme-20180715
checking for references to /build in /nix/store/b8svz8gnl07hvnppch6xd188mypnc00w-arc-theme-20180715...
/nix/store/b8svz8gnl07hvnppch6xd188mypnc00w-arc-theme-20180715

xeji merged commit ca3231a into NixOS:master

Contributor

romildo commented Sep 26, 2018

@xeji thanks.

rembo10 deleted the update-arc-theme branch

September 26, 2018 14:38

Contributor Author

rembo10 commented Sep 26, 2018

seconded!

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