Alfonso Sanchez-Beato has proposed merging 
~alfonsosanchezbeato/network-manager:better-eth-handling into 
network-manager:snap-1.10.

Commit message:
* Set sensible defaults for CFLAGS
* Add option to (un)set NM as default netplan renderer
* Remove ethernet.enable setting
* Check major series version to select DNS resolver
* Fix some shellcheck warnings

Requested reviews:
  Network-manager (network-manager)

For more details, see:
https://code.launchpad.net/~alfonsosanchezbeato/network-manager/+git/network-manager/+merge/363488

* Set sensible defaults for CFLAGS
* Add option to (un)set NM as default netplan renderer
* Remove ethernet.enable setting
* Check major series version to select DNS resolver
* Fix some shellcheck warnings
-- 
Your team Network-manager is requested to review the proposed merge of 
~alfonsosanchezbeato/network-manager:better-eth-handling into 
network-manager:snap-1.10.
diff --git a/snap-common/bin/dhcp-lease-mover b/snap-common/bin/dhcp-lease-mover
index c54aeb0..a6778da 100755
--- a/snap-common/bin/dhcp-lease-mover
+++ b/snap-common/bin/dhcp-lease-mover
@@ -4,23 +4,23 @@ set -x
 lease_path=$SNAP_DATA/state/dhcp
 public_lease_path=/run/NetworkManager/dhcp/
 
-if [ ! -e $public_lease_path ] ; then
-    mkdir -p $public_lease_path
+if [ ! -e "$public_lease_path" ] ; then
+    mkdir -p "$public_lease_path"
 fi
 
-if [ ! -e $lease_path ] ; then
-    mkdir -p $lease_path
+if [ ! -e "$lease_path" ] ; then
+    mkdir -p "$lease_path"
 fi
 
 # Copy all leases when we start to make sure we're in sync
-rm -f $public_lease_path/*
-cp $lease_path/* $public_lease_path
+rm -f "$public_lease_path"/*
+cp "$lease_path"/* "$public_lease_path"
 
 # Now we wait until a lease changes, gets added or removed and when
 # that happened we simply just move all leases files into the public
 # location.
-while $SNAP/usr/bin/inotifywait -e create,modify,delete,move $lease_path ; do
+while "$SNAP"/usr/bin/inotifywait -e create,modify,delete,move "$lease_path" ; do
     sleep 1
-    rm -f $public_lease_path/*
-    cp $lease_path/* $public_lease_path
+    rm -f "$public_lease_path"/*
+    cp "$lease_path"/* "$public_lease_path"
 done
diff --git a/snap-common/bin/networkmanager b/snap-common/bin/networkmanager
index be4ca09..0ee19a1 100755
--- a/snap-common/bin/networkmanager
+++ b/snap-common/bin/networkmanager
@@ -47,49 +47,12 @@ fi
 # Use right DNS settings depending on UC series
 . "$SNAP"/bin/utils.sh
 dns_conf_file="$SNAP_DATA"/conf.d/10-dns.conf
-if [ "$(get_target_system)" = "16" ]; then
+if [ "$(get_series_major_version)" = "16" ]; then
     printf "[main]\ndns=default\nrc-manager=resolvconf\n" > "$dns_conf_file"
 else
     printf "[main]\ndns=systemd-resolved\n" > "$dns_conf_file"
 fi
 
-# If netplan is not configured to render by default to NetworkManager
-# configuration files we disable management of any ethernet device
-# as this will clash with any configuration netplan puts in place
-# for networkd.
-if [ ! -e "/etc/netplan/00-default-nm-renderer.yaml" ] ; then
-    if [ ! -e "$SNAP_DATA"/conf.d/disable-ethernet.conf ] ; then
-        echo "[keyfile]" > "$SNAP_DATA"/conf.d/disable-ethernet.conf
-        echo "unmanaged-devices+=interface-name:eth*,interface-name:enx*" >> \
-             "$SNAP_DATA"/conf.d/disable-ethernet.conf
-    fi
-else
-    # Enable ethernet management again if the user switched the netplan
-    # backend and wants us to manage ethernet
-    if [ -e "$SNAP_DATA"/conf.d/disable-ethernet.conf ] ; then
-        rm -f "$SNAP_DATA"/conf.d/disable-ethernet.conf
-    fi
-
-    # If the snapd configuration for netplan is not present or empty
-    # we will start managing all ethernet ports automatically. Because
-    # of that we need to remove any previously created configuration files
-    # which prevented us from doing that.
-    if [ ! -e /etc/netplan/00-snapd-config.yaml ] ||
-           [ -s /etc/netplan/00-snapd-config.yaml ] ; then
-        rm -f "$SNAP_DATA"/conf.d/no-auto-default-ethernet.conf
-    else
-        if [ ! -e "$SNAP_DATA"/conf.d/no-auto-default-ethernet.conf ] ; then
-            # If we're running as the only network management service
-            # and are configured via netplan on first boot then we should
-            # not try to auto configure ethernet ports as this is up to
-            # netplan and will be the same for networkd.
-            echo "[main]" > "$SNAP_DATA"/conf.d/no-auto-default-ethernet.conf
-            echo "no-auto-default=interface-name:eth*,interface-name:enx*" >> \
-                 "$SNAP_DATA"/conf.d/no-auto-default-ethernet.conf
-        fi
-    fi
-fi
-
 # HACK: Until we've fixed probert to look in $SNAP_DATA/state/dhcp or
 # somewhere else for our lease files we use inotifywatch to monitor
 # our lease files and copy all over when something has changed. This
diff --git a/snap-common/bin/snap-config.sh b/snap-common/bin/snap-config.sh
index 7ca830a..1c61ddc 100644
--- a/snap-common/bin/snap-config.sh
+++ b/snap-common/bin/snap-config.sh
@@ -111,24 +111,6 @@ _switch_wifi_wake_on_wlan() {
     _replace_file_if_diff "$path" "$content"
 }
 
-# Change netplan renderer to NM
-# $1: true/false literal string
-_switch_ethernet() {
-    path=/etc/netplan/00-default-nm-renderer.yaml
-    case "$1" in
-        true)
-            content=$(printf "network:\n  renderer: NetworkManager")
-            _replace_file_if_diff "$path" "$content"
-            ;;
-        false)
-            rm -f "$path"
-            ;;
-        *)
-            echo "WARNING: Invalid value provided for ethernet"
-            return
-    esac
-}
-
 # Enable debug mode
 # $1: true/false literal string
 _switch_debug_enable() {
@@ -171,14 +153,29 @@ _switch_connectivity_check() {
     _replace_file_if_diff "$path" "$content"
 }
 
+# Set/unset NM as default netplan renderer
+# $1: true/false
+_switch_defaultrenderer() {
+    path=/etc/netplan/00-default-nm-renderer.yaml
+    if [ "$1" = true ] || [ "$1" = yes ]; then
+        if [ ! -f "$path" ]; then
+            printf "network:\n  renderer: NetworkManager\n" > "$path"
+            # TODO When implemented by snapd, call netplan apply
+        fi
+    elif [ -f "$path" ]; then
+        rm -f "$path"
+        # TODO When implemented by snapd, call netplan apply
+    fi
+}
+
 . "$SNAP"/bin/snap-prop.sh
 
 apply_snap_config() {
     _switch_wifi_powersave "$(get_wifi_powersave)"
     _switch_wifi_wake_on_wlan "$(get_wifi_wake_on_wlan)" "$(get_wifi_wake_on_password)"
-    _switch_ethernet "$(get_ethernet_enable)"
     _switch_debug_enable "$(get_debug_enable)"
     _switch_connectivity_check "$(get_property connectivity.uri)" \
                                "$(get_property connectivity.interval)" \
                                "$(get_property connectivity.response)"
+    _switch_defaultrenderer "$(get_defaultrenderer)"
 }
diff --git a/snap-common/bin/snap-prop.sh b/snap-common/bin/snap-prop.sh
index 3a0ec11..4a6e409 100644
--- a/snap-common/bin/snap-prop.sh
+++ b/snap-common/bin/snap-prop.sh
@@ -42,25 +42,18 @@ get_wifi_wake_on_password() {
     snapctl get wifi.wake-on-wlan-password || true
 }
 
-get_ethernet_enable() {
-    value=$(snapctl get ethernet.enable) || true
+get_debug_enable() {
+    value=$(snapctl get debug.enable) || true
     if [ -z "$value" ]; then
-        # If this file was already present, assume NM is wanted to handle
-        # ethernet in the device. Ideally this should be handled by setting
-        # NM's ethernet.enable property in the gadget snap though.
-        if [ -e /etc/netplan/00-default-nm-renderer.yaml ]; then
-            value=true
-        else
-            value=false
-        fi
+        value=false
     fi
     echo "$value"
 }
 
-get_debug_enable() {
-    value=$(snapctl get debug.enable) || true
+get_defaultrenderer() {
+    value=$(snapctl get defaultrenderer) || true
     if [ -z "$value" ]; then
-        value=false
+        value=true
     fi
     echo "$value"
 }
diff --git a/snap-common/bin/utils.sh b/snap-common/bin/utils.sh
index 713b6a3..d449638 100644
--- a/snap-common/bin/utils.sh
+++ b/snap-common/bin/utils.sh
@@ -2,16 +2,6 @@
 
 . /etc/lsb-release
 
-get_target_system() {
-    value=$(cat /proc/cmdline)
-
-    case "$value" in
-        *snap_core*)
-            case "$DISTRIB_RELEASE" in
-                *18*) echo "18" ;;
-                *) echo "16" ;;
-            esac
-	    ;;
-        *) echo "classic" ;;
-    esac
+get_series_major_version() {
+    printf "%s\n" "${DISTRIB_RELEASE%%.*}"
 }
diff --git a/snap-common/startup-hooks/99-wol-by-default.sh b/snap-common/startup-hooks/99-wol-by-default.sh
index c2d4480..8bc456a 100755
--- a/snap-common/startup-hooks/99-wol-by-default.sh
+++ b/snap-common/startup-hooks/99-wol-by-default.sh
@@ -2,9 +2,9 @@
 
 # Enable wake-on-lan by default until we have a configuration
 # hook to do that.
-if [ ! -e $SNAP_DATA/conf.d/enable-wol.conf ] ; then
-	mkdir -p $SNAP_DATA/conf.d
-	cat <<-EOF > $SNAP_DATA/conf.d/enable-wol.conf
+if [ ! -e "$SNAP_DATA"/conf.d/enable-wol.conf ] ; then
+	mkdir -p "$SNAP_DATA"/conf.d
+	cat <<-EOF > "$SNAP_DATA"/conf.d/enable-wol.conf
 		[connection]
 		# Value 64 maps to the 'magic' setting; see man nm-settings
 		# for more information.
diff --git a/snap/hooks/configure b/snap/hooks/configure
index 34b41a6..6237497 100755
--- a/snap/hooks/configure
+++ b/snap/hooks/configure
@@ -26,31 +26,31 @@ _create_props_content() {
     printf "wifi_powersave=%s\n" "$wifi_powersave"
     printf "wifi_wake_on_wlan=%s\n" "$wifi_wake_on_wlan"
     printf "wifi_wake_on_password=%s\n" "$wifi_wake_on_password"
-    printf "ethernet_enable=%s\n" "$ethernet_enable"
     printf "debug_enable=%s\n" "$debug_enable"
     printf "connectivity_uri=%s\n" "$connectivity_uri"
     printf "connectivity_interval=%s\n" "$connectivity_interval"
     printf "connectivity_response=%s\n" "$connectivity_response"
+    printf "defaultrenderer=%s\n" "$defaultrenderer"
 }
 
 wifi_powersave=$(get_wifi_powersave)
 wifi_wake_on_wlan=$(get_wifi_wake_on_wlan)
 wifi_wake_on_password=$(get_wifi_wake_on_password)
-ethernet_enable=$(get_ethernet_enable)
 debug_enable=$(get_debug_enable)
 connectivity_uri=$(get_property "connectivity.uri")
 connectivity_interval=$(get_property "connectivity.interval")
 connectivity_response=$(get_property "connectivity.response")
+defaultrenderer=$(get_defaultrenderer)
 
 # Store always, so we show defaults when properties are set to empty strings
 snapctl set wifi.powersave="$wifi_powersave"
 snapctl set wifi.wake-on-wlan="$wifi_wake_on_wlan"
 snapctl set wifi.wake-on-wlan-password="$wifi_wake_on_password"
-snapctl set ethernet.enable="$ethernet_enable"
 snapctl set debug.enable="$debug_enable"
 snapctl set connectivity.uri="$connectivity_uri"
 snapctl set connectivity.interval="$connectivity_interval"
 snapctl set connectivity.response="$connectivity_response"
+snapctl set defaultrenderer="$defaultrenderer"
 
 content=$(_create_props_content)
 
diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
index d9e1363..068686f 100644
--- a/snap/snapcraft.yaml
+++ b/snap/snapcraft.yaml
@@ -161,6 +161,8 @@ parts:
       - --disable-more-warnings
       - --disable-modify-system
       - --disable-ovs
+      # Set explicitly CFLAGS until lp: #1791946 is solved
+      - CFLAGS=-O2 -g
     override-build: |
       echo "networkmanager override-build called <patches go here>"
       export QUILT_PATCHES=debian/patches
@@ -170,8 +172,6 @@ parts:
       rm ./configure
       snapcraftctl build
 
-      # Run all tests NetworkManager ships by default
-      make check
     stage-packages:
       - iputils-arping
       - libasn1-8-heimdal
-- 
ubuntu-desktop mailing list
ubuntu-desktop@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-desktop

Reply via email to