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
Merged

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

merged 1 commit into from May 11, 2019

Conversation

romildo
Copy link
Contributor

@romildo romildo 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.

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.

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
Copy link
Contributor Author

romildo 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
Copy link
Contributor Author

romildo 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
Copy link
Contributor

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
Copy link
Contributor

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

@romildo
Copy link
Contributor Author

romildo 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 merged commit 76e3af4 into NixOS:master May 11, 2019
@romildo romildo deleted the fix.deepin.deepin-wm branch May 11, 2019 02:36
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