(#1762) Allow shells to enable XWayland by default (#3133)

* Add option to enable XWayland by default * Use a regular member function instead of constructor; update symbols * Move new symbol back to 4.1 * Move new symbol back to 4.1 * Add notice about the breaking change --------- Co-authored-by: Alan Griffiths <[email protected]>
canonical · Nov 22, 2023 · f51d081 · f51d081
1 parent a75b8b3
commit f51d081
Show file tree

Hide file tree

Showing 8 changed files with 61 additions and 16 deletions.
diff --git a/debian/libmiral6.symbols b/debian/libmiral6.symbols
@@ -424,3 +424,4 @@ libmiral.so.6 libmiral6 #MINVER#
  MIRAL_4.1@MIRAL_4.1 4.1.0
  (c++)"miral::WaylandExtensions::zwp_input_method_v1@MIRAL_4.1" 4.1.0
  (c++)"miral::WaylandExtensions::zwp_input_panel_v1@MIRAL_4.1" 4.1.0
+ (c++)"miral::X11Support::default_to_enabled()@MIRAL_4.1" 4.1.0
diff --git a/include/miral/miral/x11_support.h b/include/miral/miral/x11_support.h
@@ -23,8 +23,10 @@ namespace mir { class Server; }
 
 namespace miral
 {
-/// Add a user configuration option for X11 support.
+/// Add user configuration options for X11 support.
 /// \remark Since MirAL 2.4
+/// \note From MirAL 4.1, the \c --enable-x11 option requires an argument
+///       (e.g., \c --enable-x11=true to enable X11 support).
 class X11Support
 {
 public:
@@ -36,6 +38,15 @@ class X11Support
     X11Support(X11Support const&);
     auto operator=(X11Support const&) -> X11Support&;
 
+    /**
+     * Set X11 support as enabled by default.
+     *
+     * \return A reference to the modified miral::X11Support object with
+     *         the updated default configuration.
+     * \remark Since MirAL 4.1
+     */
+    auto default_to_enabled() -> X11Support&;
+
 private:
     struct Self;
     std::shared_ptr<Self> self;

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -51,4 +51,3 @@ set(MIR_SERVER_GRAPHICS_PLATFORM_ABI ${MIR_SERVER_GRAPHICS_PLATFORM_ABI} PARENT_
 set(MIR_SERVER_INPUT_PLATFORM_ABI ${MIR_SERVER_INPUT_PLATFORM_ABI} PARENT_SCOPE)
 set(MIR_SERVER_GRAPHICS_PLATFORM_VERSION ${MIR_SERVER_GRAPHICS_PLATFORM_VERSION} PARENT_SCOPE)
 set(MIR_INPUT_PLATFORM_VERSION_SCRIPT ${MIR_INPUT_PLATFORM_VERSION_SCRIPT} PARENT_SCOPE)
-
diff --git a/src/miral/symbols.map b/src/miral/symbols.map
@@ -478,6 +478,7 @@ global:
   extern "C++" {
     miral::WaylandExtensions::zwp_input_method_v1*;
     miral::WaylandExtensions::zwp_input_panel_v1*;
+    miral::X11Support::default_to_enabled*;
   };
 
 } MIRAL_4.0;

diff --git a/src/miral/x11_support.cpp b/src/miral/x11_support.cpp
@@ -26,6 +26,7 @@ namespace mo = mir::options;
 
 struct miral::X11Support::Self
 {
+    bool x11_enabled{false};
 };
 
 miral::X11Support::X11Support() : self{std::make_shared<Self>()}
@@ -38,8 +39,7 @@ void miral::X11Support::operator()(mir::Server& server) const
 
     server.add_configuration_option(
         mo::x11_display_opt,
-        "Enable X11 support", mir::OptionType::null);
-
+        "Enable X11 support [{1|true|on|yes, 0|false|off|no}].", self->x11_enabled);
 
     auto const x11_scale_default = []() -> char const* {
         if (auto const gdk_scale = getenv("GDK_SCALE"))
@@ -96,4 +96,9 @@ miral::X11Support::~X11Support() = default;
 miral::X11Support::X11Support(X11Support const&) = default;
 auto miral::X11Support::operator=(X11Support const&) -> X11Support& = default;
 
+auto miral::X11Support::default_to_enabled() -> X11Support&
+{
+    self->x11_enabled = true;
 
+    return *this;
+}
diff --git a/src/server/frontend_wayland/wayland_default_configuration.cpp b/src/server/frontend_wayland/wayland_default_configuration.cpp
@@ -332,6 +332,7 @@ std::shared_ptr<mf::Connector>
                 enabled_wayland_extensions.end()};
 
             auto const enable_repeat = options->get<bool>(options::enable_key_repeat_opt);
+            auto const x11_enabled = options->is_set(mo::x11_display_opt) && options->get<bool>(mo::x11_display_opt);
 
             return std::make_shared<mf::WaylandConnector>(
                 the_shell(),
@@ -356,7 +357,7 @@ std::shared_ptr<mf::Connector>
                 arw_socket,
                 configure_wayland_extensions(
                     wayland_extensions,
-                    options->is_set(mo::x11_display_opt),
+                    x11_enabled,
                     wayland_extension_hooks),
                 wayland_extension_filter,
                 enable_repeat);

diff --git a/src/server/frontend_xwayland/xwayland_default_configuration.cpp b/src/server/frontend_xwayland/xwayland_default_configuration.cpp
@@ -65,7 +65,9 @@ std::shared_ptr<mf::Connector> mir::DefaultServerConfiguration::the_xwayland_con
     return xwayland_connector([this]() -> std::shared_ptr<mf::Connector> {
 
         auto options = the_options();
-        if (options->is_set(mo::x11_display_opt))
+        auto const x11_enabled = options->is_set(mo::x11_display_opt) && options->get<bool>(mo::x11_display_opt);
+
+        if (x11_enabled)
         {
             try
             {

diff --git a/tests/miral/external_client.cpp b/tests/miral/external_client.cpp
@@ -40,7 +40,8 @@ struct ExternalClient : miral::TestServer
     }
 
     miral::ExternalClientLauncher external_client;
-    miral::X11Support x11;
+    miral::X11Support x11_disabled_by_default{};
+    miral::X11Support x11_enabled_by_default{miral::X11Support{}.default_to_enabled()};
 
     std::string const output = tmpnam(nullptr);
 
@@ -66,7 +67,7 @@ struct ExternalClient : miral::TestServer
         getline(in, result);
         return result;
     }
-    
+
     bool cannot_start_X_server()
     {
         // Starting an X server on LP builder, or Fedora CI, doesn't work
@@ -101,7 +102,7 @@ TEST_F(ExternalClient, default_app_env_x11_is_as_expected)
     if (cannot_start_X_server())
         return; // Starting an X server doesn't work - skip the test
 
-    add_server_init(x11);
+    add_server_init(x11_disabled_by_default);
     add_to_environment("MIR_SERVER_ENABLE_X11", "");
     start_server();
 
@@ -126,7 +127,7 @@ TEST_F(ExternalClient, override_app_env_x11_can_unset)
         return; // Starting an X server doesn't work - skip the test
 
     add_to_environment(app_x11_env, "-GDK_BACKEND");
-    add_server_init(x11);
+    add_server_init(x11_disabled_by_default);
     add_to_environment("MIR_SERVER_ENABLE_X11", "");
     start_server();
 
@@ -139,7 +140,7 @@ TEST_F(ExternalClient, override_app_env_x11_can_unset_and_set)
         return; // Starting an X server doesn't work - skip the test
 
     add_to_environment(app_x11_env, "-GDK_BACKEND:QT_QPA_PLATFORM=xcb");
-    add_server_init(x11);
+    add_server_init(x11_disabled_by_default);
     add_to_environment("MIR_SERVER_ENABLE_X11", "");
     start_server();
 
@@ -153,7 +154,7 @@ TEST_F(ExternalClient, override_app_env_x11_can_set_and_unset)
         return; // Starting an X server doesn't work - skip the test
 
     add_to_environment(app_x11_env, "QT_QPA_PLATFORM=xcb:-GDK_BACKEND");
-    add_server_init(x11);
+    add_server_init(x11_disabled_by_default);
     add_to_environment("MIR_SERVER_ENABLE_X11", "");
     start_server();
 
@@ -167,7 +168,7 @@ TEST_F(ExternalClient, stray_separators_are_ignored)
         return; // Starting an X server doesn't work - skip the test
 
     add_to_environment(app_x11_env, "::QT_QPA_PLATFORM=xcb::-GDK_BACKEND::");
-    add_server_init(x11);
+    add_server_init(x11_disabled_by_default);
     add_to_environment("MIR_SERVER_ENABLE_X11", "");
     start_server();
 
@@ -181,7 +182,7 @@ TEST_F(ExternalClient, empty_override_does_nothing)
         return; // Starting an X server doesn't work - skip the test
 
     add_to_environment(app_x11_env, "");
-    add_server_init(x11);
+    add_server_init(x11_disabled_by_default);
     add_to_environment("MIR_SERVER_ENABLE_X11", "");
     start_server();
 
@@ -198,7 +199,7 @@ TEST_F(ExternalClient, strange_override_does_nothing)
         return; // Starting an X server doesn't work - skip the test
 
     add_to_environment(app_x11_env, "=====");
-    add_server_init(x11);
+    add_server_init(x11_disabled_by_default);
     add_to_environment("MIR_SERVER_ENABLE_X11", "");
     start_server();
 
@@ -215,7 +216,7 @@ TEST_F(ExternalClient, another_strange_override_does_nothing)
         return; // Starting an X server doesn't work - skip the test
 
     add_to_environment(app_x11_env, ":::");
-    add_server_init(x11);
+    add_server_init(x11_disabled_by_default);
     add_to_environment("MIR_SERVER_ENABLE_X11", "");
     start_server();
 
@@ -244,3 +245,27 @@ TEST_F(ExternalClient, split_command)
             << "test.command=\"" + test.command + "\"";
     }
 }
+
+TEST_F(ExternalClient, x11_disabled_by_default_is_as_expected)
+{
+    if (cannot_start_X_server())
+        return; // Starting an X server doesn't work - skip the test
+
+    add_server_init(x11_disabled_by_default);
+    start_server();
+
+    auto result = external_client.launch_using_x11({"bash", "-c", "exit"});
+    EXPECT_THAT(result, Eq(-1));
+}
+
+TEST_F(ExternalClient, x11_enabled_by_default_is_as_expected)
+{
+    if (cannot_start_X_server())
+        return; // Starting an X server doesn't work - skip the test
+
+    add_server_init(x11_enabled_by_default);
+    start_server();
+
+    auto result = external_client.launch_using_x11({"bash", "-c", "exit"});
+    EXPECT_THAT(result, Ne(-1));
+}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -51,4 +51,3 @@ set(MIR_SERVER_GRAPHICS_PLATFORM_ABI ${MIR_SERVER_GRAPHICS_PLATFORM_ABI} PARENT_
		set(MIR_SERVER_INPUT_PLATFORM_ABI ${MIR_SERVER_INPUT_PLATFORM_ABI} PARENT_SCOPE)
		set(MIR_SERVER_GRAPHICS_PLATFORM_VERSION ${MIR_SERVER_GRAPHICS_PLATFORM_VERSION} PARENT_SCOPE)
		set(MIR_INPUT_PLATFORM_VERSION_SCRIPT ${MIR_INPUT_PLATFORM_VERSION_SCRIPT} PARENT_SCOPE)