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

Unified network change PRs with additional debian/ fix #7486

Closed
wants to merge 9 commits into from

Conversation

Projects
None yet
9 participants
@Fantu
Copy link
Contributor

commented Apr 20, 2018

This include #6993 and #7118 and require/include #7506

@Fantu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2018

Found another issue with circleci tests: gir1.2-nm-1.0 is missed in stretch (my tests on stretch-backport with updated network-manager from unofficial backport not helped to saw it), now I'll check for solution
gir1.2-nm-1.0 was splitted in 1.8.0-2

@linuxmint linuxmint deleted a comment from codacy-bot Apr 20, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 20, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 20, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 20, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 20, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 20, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 20, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 20, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 20, 2018

@Fantu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2018

tried today on stretch with cinnamon 3.6 debian packages backport including this PR and cinnamon crash, tried to rebuild network-manager with NetworkManager/NetworkManager@b2af5f7 but still crash and I was unable to debug the exact cause of issue because the backtrace seems not directly related to it but its conseguence :(

@clefebvre

This comment has been minimized.

Copy link
Member

commented Apr 24, 2018

OK, we're getting closer.

@Fantu can you rebase to solve the conflict on debian/control?

I found the source of the issue (crash when network applet is running):

https://git.gnome.org/browse/network-manager-applet/commit/?id=7a59d41e5f6666d0da51f1f7aae7518befdb1182

This issue is present in Debian stretch. It makes gir1.2-nma use the old libs (NMClient/glib) instead of libnm. This results in making it impossible to use NM and NMA gir at the same time.

In Cinnamon libnma is used via gir in modemManager.js.

@clefebvre

This comment has been minimized.

Copy link
Member

commented Apr 24, 2018

So, to summarize the state of this PR:

In Mint 19 and distros with recent versions of NM, everything's working fine, no problem at all.

In LMDE 3 and Debian (with NM 1.6), everything's working after applying two patches:

@leigh123linux

This comment has been minimized.

Copy link
Member

commented Apr 24, 2018

So Debian is broken by design?, why should we accommodate their frozen broken packages?
It is their choice to ship old un-patched versions.

@Fantu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2018

I also think is not good :(
I opened bugs about but need someone explain better:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=892998 (this I also not updated that not is the cause of crash)
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=896818

@leigh123linux

This comment has been minimized.

Copy link
Member

commented Apr 24, 2018

@Fantu Do debian maintainers like a challenge?, it took redhat 4 months to fix.

https://bugzilla.redhat.com/show_bug.cgi?id=1520398

Can debian do better than an enterprise distro?

@clefebvre

This comment has been minimized.

Copy link
Member

commented Apr 24, 2018

We just need to illustrate the issues properly and show that they're specific to Debian's versions of libnm and libnma and affect any Debian developer, not just some random future version of Cinnamon which Debian doesn't ship.

I did this here for libNMA:

https://gist.github.com/clefebvre/411ab17558fc4dc438085ce2b6cc9093

and here for libNM:

https://gist.github.com/clefebvre/c957c3eb290116d07aa524e97743a79e

@Fantu can you show these explanations to Michael Biebl biebl@debian.org. They speak for themselves and they're isolating both issues.

Then the process is easy:

  • Michael gets these issues patched in Debian Stretch
  • libNM becomes a library which works in all major distros
  • we migrate to it with this PR
  • we tag the move in a 3.8.x point release
  • everybody's happy and everything works everywhere
@maxyz

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2018

I don't think that we have to wait for a solution of this issue in stretch, if that ever happens, it's going to take a while, it needs approval of the stable release team, and the package will only be in proposed updates till the next point release.
I understand that this might be a bit a hassle for lmde3, but it might be simpler and faster to ship network-manager package (if you use a version 1.6.2-3+~lmde3 then it would be possible for a 1.6.2-3+deb9 package to update it) with the mentioned patches there.

debian/control: increase deps version of muffin, cjs and cinnamon-des…
…ktop

make them strict in packages to avoid unexpected cases
build-deps instead increased to latest (is not possible use ${cinnamon:Version}
on it)

@Fantu Fantu force-pushed the Fantu:network-test branch from 86d3937 to eb5506c Apr 26, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 26, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 26, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 26, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 26, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 26, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 26, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 27, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 27, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 27, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 27, 2018

@linuxmint linuxmint deleted a comment from codacy-bot Apr 27, 2018

mtwebster added some commits Apr 27, 2018

network applet: Don't show unmanaged networks in the list.
We don't display them in our networking configuration pages, and
we cannot do anything with them, so there's no reason for them in
the applet.

Fixes #5726

Note:
this is a fast libnm migration of the corrispective commit in master
network applet: refresh the panel icon more frequently when the menu
is open.

Signal strength for individual AP's, including the connected one, are
updated as the underlying device reports those changes.  The panel icon,
however, is on a fixed update schedule - updating only every 10 seconds
except when a connection is in the process of being made.

There's nothing wrong here really, except when you have the applet menu
open, the active connection's strength can be at odds with the icon in the
panel.  This will cause updates to the panel icon to happen every 2 seconds
when the menu is open, and revert to every 10 seconds when it's not.

Refactor additionally to prevent a new GSource being created during each
periodic update.

Note:
this is a fast libnm migration of the corrispective commit in master
network applet: show the actual active connection's signal strength
in the AP list, rather than the strongest AP for a given ssid, which
may not be the same thing.

Remove the periodic timer for the panel icon and text updates.  Reacting
to a connection's strength changes directly results in an average of
around 10.5s between updates (measured over about 1 hour,) which is
slightly better than our periodic update, and has the added advantages
of our signal strength always being immediately updated when the underlying
property changes, and not having the additional overhead and logic of an
extra timer.

Note:
this is a fast libnm migration of the corrispective commit in master

@linuxmint linuxmint deleted a comment from codacy-bot May 2, 2018

@linuxmint linuxmint deleted a comment from codacy-bot May 2, 2018

@linuxmint linuxmint deleted a comment from codacy-bot May 2, 2018

@@ -485,25 +476,25 @@ NMDevice.prototype = {

get statusLabel(){
switch(this.device.state) {
case NetworkManager.DeviceState.DISCONNECTED:
case NetworkManager.DeviceState.ACTIVATED:
case NM.DeviceState.DISCONNECTED:

This comment has been minimized.

@linuxmint linuxmint deleted a comment from codacy-bot May 2, 2018

@linuxmint linuxmint deleted a comment from codacy-bot May 2, 2018

@@ -525,7 +516,7 @@ NMDevice.prototype = {
/* Translators: this is for a network device that cannot be activated (for example it
is disabled by rfkill, or it has no coverage */
return _("unavailable");
case NetworkManager.DeviceState.FAILED:
case NM.DeviceState.FAILED:

This comment has been minimized.

return null;
case NetworkManager.DeviceState.UNMANAGED:
case NM.DeviceState.UNMANAGED:

This comment has been minimized.

@linuxmint linuxmint deleted a comment from codacy-bot May 2, 2018

@linuxmint linuxmint deleted a comment from codacy-bot May 2, 2018

case NetworkManager.DeviceState.DISCONNECTED:
case NetworkManager.DeviceState.ACTIVATED:
case NM.DeviceState.DISCONNECTED:
case NM.DeviceState.ACTIVATED:

This comment has been minimized.

@linuxmint linuxmint deleted a comment from codacy-bot May 2, 2018

@linuxmint linuxmint deleted a comment from codacy-bot May 2, 2018

case NetworkManager.DeviceState.SECONDARIES:
case NM.DeviceState.PREPARE:
case NM.DeviceState.CONFIG:
case NM.DeviceState.IP_CONFIG:

This comment has been minimized.

@@ -514,7 +505,7 @@ NMDevice.prototype = {
module, which is missing */
return _("firmware missing");
}
if (this.device.capabilities & NetworkManager.DeviceCapabilities.CARRIER_DETECT) {
if (this.device.capabilities & NM.DeviceCapabilities.CARRIER_DETECT) {

This comment has been minimized.

@codacy-bot

This comment has been minimized.

Copy link

commented May 2, 2018

Codacy Here is an overview of what got changed by this pull request:

Issues
======
+ Solved 9
- Added 9
           

Complexity decreasing per file
==============================
+ files/usr/share/cinnamon/applets/network@cinnamon.org/applet.js  -1
         

Clones added
============
- files/usr/share/cinnamon/applets/network@cinnamon.org/applet.js  2
         

See the complete overview on Codacy

case NM.DeviceState.CONFIG:
case NM.DeviceState.IP_CONFIG:
case NM.DeviceState.IP_CHECK:
case NM.DeviceState.SECONDARIES:

This comment has been minimized.

/* Translators: this is for network devices that are physically present but are not
under NetworkManager's control (and thus cannot be used in the menu) */
return _("unmanaged");
case NetworkManager.DeviceState.DEACTIVATING:
case NM.DeviceState.DEACTIVATING:

This comment has been minimized.

return _("connecting...");
case NetworkManager.DeviceState.NEED_AUTH:
case NM.DeviceState.NEED_AUTH:

This comment has been minimized.

@Fantu

This comment has been minimized.

Copy link
Contributor Author

commented May 4, 2018

@clem Watch #7536 instead that is better (rebased), also include a change commit I missed here and add also a fix for a bug introduced in 3.8.1 network changes

@Fantu Fantu closed this May 7, 2018

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.