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

plasma-nm: fix path to mobile broadband provider file #44302

Merged
merged 1 commit into from Aug 1, 2018

Conversation

Yarny0
Copy link
Contributor

@Yarny0 Yarny0 commented Aug 1, 2018

When creating a new mobile broadband connection
with the plasma network manager connection editor,
it tries to find a file containing provider
information somewhere in /usr/share/... .
The build recipe contains a patch to fix the lookup path
such that it finds the file in the corresponding package,
probably added due to
#9389 .
The actual lookup path is injected into
the patch file with substituteAll.

With commit a31d98f ,
the variable name used in subsituteAll changed from
mobile_broadband_provider_info to mobile-broadband-provider-info
(underscores in package names turned into dashes).
Apparently, substituteAll can't handle dashes in variable names.
Consequently, the variable name was no longer resolved.
plasma-nm failed to create new mobile broadband connections;
the connection creator silently exited and logged the error

plasma-nm: Error opening providers file "@mobile-broadband-provider-info@/share/mobile-broadband-provider-info/serviceproviders.xml"

This commit keeps the dashes in package names, but it
restores the underscores in the variable used by substituteAll,
thereby ensuring the variable gets resolved properly.

Motivation for this change

Restore ability to create mobile broadband connections with the plasma network manager frontend.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS: on 18.03
    • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/): there are none
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

cc @ttuegel (sorry, you are the last one who touched this according to git log ... there is no maintainer given in the build recipe)

When creating a new mobile broadband connection
with the plasma network manager connection editor,
it tries to find a file containing provider
information somewhere in /usr/share/... .
The build recipe contains a patch to fix the lookup path
such that it finds the file in the corresponding package,
probably added due to
NixOS#9389 .
The actual lookup path is injected into
the patch file with substituteAll.

With commit a31d98f ,
the variable name used in subsituteAll changed from
mobile_broadband_provider_info to mobile-broadband-provider-info
(underscores in package names turned into dashes).
Apparently, substituteAll can't handle dashes in variable names.
Consequently, the variable name was no longer resolved.
plasma-nm failed to create new mobile broadband connections;
the connection creator silently exited and logged the error
> plasma-nm: Error opening providers file "@mobile-broadband-provider-info@/share/mobile-broadband-provider-info/serviceproviders.xml"

This commit keeps the dashes in package names, but it
restores the underscores in the variable used by substituteAll,
thereby ensuring the variable gets resolved properly.
@jtojnar
Copy link
Contributor

jtojnar commented Aug 1, 2018

Bah, this has bitten me in the rear multiple times. Maybe substituteAll should print a warning when a variable that is not a valid bash variable is passed.

@xeji
Copy link
Contributor

xeji commented Aug 1, 2018

@GrahamcOfBorg build plasma-nm

@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: plasma-nm

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.


@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: plasma-nm

Partial log (click to expand)

shrinking /nix/store/b6cq2p4ya0c2nfjfhc165h4prsmwxn6c-plasma-nm-5.13.2/lib/qt-5.11/plugins/libplasmanetworkmanagement_iodineui.so
shrinking /nix/store/b6cq2p4ya0c2nfjfhc165h4prsmwxn6c-plasma-nm-5.13.2/lib/qt-5.11/plugins/libplasmanetworkmanagement_fortisslvpnui.so
shrinking /nix/store/b6cq2p4ya0c2nfjfhc165h4prsmwxn6c-plasma-nm-5.13.2/lib/qt-5.11/plugins/kcm_networkmanagement.so
shrinking /nix/store/b6cq2p4ya0c2nfjfhc165h4prsmwxn6c-plasma-nm-5.13.2/lib/qt-5.11/plugins/kf5/kded/networkmanagement.so
strip is /nix/store/zrs21zqcchgyabjf4xfimncdq16njizc-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/b6cq2p4ya0c2nfjfhc165h4prsmwxn6c-plasma-nm-5.13.2/lib
patching script interpreter paths in /nix/store/b6cq2p4ya0c2nfjfhc165h4prsmwxn6c-plasma-nm-5.13.2
checking for references to /build in /nix/store/b6cq2p4ya0c2nfjfhc165h4prsmwxn6c-plasma-nm-5.13.2...
postPatchMkspecs
postPatchMkspecs

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: plasma-nm

Partial log (click to expand)

shrinking /nix/store/hmwcqfzg3wraw0yg7qgzpxbc3rvycw3m-plasma-nm-5.13.2/lib/qt-5.11/plugins/libplasmanetworkmanagement_l2tpui.so
shrinking /nix/store/hmwcqfzg3wraw0yg7qgzpxbc3rvycw3m-plasma-nm-5.13.2/lib/qt-5.11/plugins/libplasmanetworkmanagement_vpncui.so
shrinking /nix/store/hmwcqfzg3wraw0yg7qgzpxbc3rvycw3m-plasma-nm-5.13.2/lib/qt-5.11/qml/org/kde/plasma/networkmanagement/libplasmanm_qmlplugins.so
shrinking /nix/store/hmwcqfzg3wraw0yg7qgzpxbc3rvycw3m-plasma-nm-5.13.2/lib/libplasmanm_internal.so
strip is /nix/store/1hi76hr87bd1y1q1qjk0lv8nmcjip1c8-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/hmwcqfzg3wraw0yg7qgzpxbc3rvycw3m-plasma-nm-5.13.2/lib
patching script interpreter paths in /nix/store/hmwcqfzg3wraw0yg7qgzpxbc3rvycw3m-plasma-nm-5.13.2
checking for references to /build in /nix/store/hmwcqfzg3wraw0yg7qgzpxbc3rvycw3m-plasma-nm-5.13.2...
postPatchMkspecs
postPatchMkspecs

@xeji xeji merged commit bdf6f85 into NixOS:master Aug 1, 2018
@Yarny0 Yarny0 deleted the plasmanm-fix-mobile-broadband branch August 1, 2018 13:32
@p-alik
Copy link
Contributor

p-alik commented Aug 24, 2018

systemsettings5 on 18.03 still unable to add broadband connection.

plasma-nm: Error opening providers file "@mobile-broadband-provider-info@/share/mobile-broadband-provider-info/serviceproviders.xml"

I'd like to kindly ask you how to get rid of it?

xeji pushed a commit that referenced this pull request Aug 24, 2018
When creating a new mobile broadband connection
with the plasma network manager connection editor,
it tries to find a file containing provider
information somewhere in /usr/share/... .
The build recipe contains a patch to fix the lookup path
such that it finds the file in the corresponding package,
probably added due to
#9389 .
The actual lookup path is injected into
the patch file with substituteAll.

With commit a31d98f ,
the variable name used in subsituteAll changed from
mobile_broadband_provider_info to mobile-broadband-provider-info
(underscores in package names turned into dashes).
Apparently, substituteAll can't handle dashes in variable names.
Consequently, the variable name was no longer resolved.
plasma-nm failed to create new mobile broadband connections;
the connection creator silently exited and logged the error
> plasma-nm: Error opening providers file "@mobile-broadband-provider-info@/share/mobile-broadband-provider-info/serviceproviders.xml"

This commit keeps the dashes in package names, but it
restores the underscores in the variable used by substituteAll,
thereby ensuring the variable gets resolved properly.

(cherry picked from commit bdf6f85)
@xeji
Copy link
Contributor

xeji commented Aug 24, 2018

@p-alik this fix was merged into master. I backported it to 18.03 in 59c3c4d just now, so it should be available when the channel advances.

@p-alik
Copy link
Contributor

p-alik commented Aug 25, 2018

Thank you @xeji. Unfortunately I still get the same error on 18.03.133157.fde20125199 (Impala).
I'm using the channel https://nixos.org/channels/nixos-18.03

@xeji
Copy link
Contributor

xeji commented Aug 25, 2018

That's because the channel has not advanced to include the fix yet. Please wait a little.

@p-alik
Copy link
Contributor

p-alik commented Aug 28, 2018

@xeji, 59c3c4d couldn't be advanced for the channel nixos-18.03 yet. But it successfully advanced in the channel nixos-18.03-small.
Will the commit be advanced to the channel nixos-18.03 in the feature or will it be entirely dropped?

@xeji
Copy link
Contributor

xeji commented Aug 28, 2018

Don't worry, the commit will be available in the nixos-18.03 channel when it next advances.

@p-alik
Copy link
Contributor

p-alik commented Aug 29, 2018

One more thanks!
The issue is solved for me since upgrade to nixos-18.03.133174.edd63e05d1e

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

5 participants