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

deepin.deepin-wm: fix paths related to plugins and desktop files #59940

Merged
merged 1 commit into from May 11, 2019

Conversation

Projects
None yet
3 participants
@romildo
Copy link
Contributor

commented Apr 21, 2019

Motivation for this change
  • deepin.deepin-wm: set plugins dir from environment variable
  • deepin.deepin-wm: use absolute path in desktop file

About packaging deepin for NixOS: #59023

See also #59244 (comment)

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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@worldofpeace
Copy link
Member

left a comment

Are there any deepin-wm plugins distributed or used internally in other DDE components?
This will also need a wrapper used when there's plugins provided.

@romildo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

Are there any deepin-wm plugins distributed or used internally in other DDE components?

I did not find any.

Edited: I have looked at Arch Linux, and Deepin Linux.

@romildo

This comment has been minimized.

Copy link
Contributor Author

commented May 10, 2019

Considering that there is no other DDE component that installs any plugins for deepin-wm, it may be more convenient now to not patch the source code to set plugins dir from environment variable. Also the wrapper is not needed currently.

@worldofpeace Do you agree?

@worldofpeace

This comment has been minimized.

Copy link
Member

commented May 10, 2019

Yeah, I think there's a comment somewhere from me about patching only when there's a plugin ecosystem that's useful. For example in pantheon gala isn't patched because almost all development work for plugins eventually get upstreamed. And from the looks of it this is a fork of gala.

@worldofpeace

This comment has been minimized.

Copy link
Member

commented May 10, 2019

Though I wouldn't toss the code away, it could change in the future who knows
@romildo

This comment has been minimized.

Copy link
Contributor Author

commented May 10, 2019

I am removing the deepin-wm.plugins-dir.patch from this PR, and keeping its contents in this comment, case it is needed later on.

From 028f9214a251741df89b56836fa4e8efd16c8cb2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Romildo=20Malaquias?= <malaquias@gmail.com>
Date: Fri, 19 Apr 2019 23:56:33 -0300
Subject: [PATCH] deepin.deepin-wm: set plugin dir from environment variable

---
 src/PluginManager.vala | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/PluginManager.vala b/src/PluginManager.vala
index 9ff8856..f9ac214 100644
--- a/src/PluginManager.vala
+++ b/src/PluginManager.vala
@@ -58,7 +58,11 @@ namespace Gala
 				return;
 			}
 
-			plugin_dir = File.new_for_path (Config.PLUGINDIR);
+                        string? plugin_dir_name = GLib.Environment.get_variable ("DEEPIN_WM_PLUGINS_DIR");
+                        if (plugin_dir_name == null || plugin_dir_name == "")
+                        	plugin_dir_name = Config.PLUGINDIR;
+			debug ("Plugin dir: %s", plugin_dir_name);
+			plugin_dir = File.new_for_path (plugin_dir_name);
 			if (!plugin_dir.query_exists ())
 				return;
 
-- 
2.21.0

@romildo romildo force-pushed the romildo:fix.deepin.deepin-wm branch from 0b672d4 to edc66ee May 10, 2019

@romildo romildo merged commit 76e3af4 into NixOS:master May 11, 2019

15 of 16 checks passed

deepin.deepin-wm on x86_64-darwin No attempt
Details
Evaluation Performance Report Evaluator Performance Report
Details
deepin.deepin-wm on aarch64-linux Success
Details
deepin.deepin-wm on x86_64-linux Success
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

@romildo romildo deleted the romildo:fix.deepin.deepin-wm branch May 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.