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

Pass path of ip command to openvpn. #24

Merged
merged 1 commit into from
Apr 10, 2021

Conversation

jacobly0
Copy link
Contributor

@jacobly0 jacobly0 commented Mar 19, 2021

Description
On various systems, the ip command can be at /sbin/ip, /bin/ip, or /usr/sbin/ip. The latest included openvpn seems to be using a hardcoded path of /sbin/ip, presumably determined at build time. This PR just determines the correct path and passes it to openvpn to override the hardcoded path.

Checklist

  • I have described below how to test my code and included unit tests where possible.
  • I have cleaned up the code and made best efforts to match the surrounding coding style.
  • I certify that I have the right to submit this contribution under the GPLv3 and alter as outlined in the Contributor License Agreement.

Testing Plan
From what I understand at least Fedora 33 and CentOS use /usr/sbin/ip, and so this should fix a failure to connect on those systems.

@JonathonH-PIA
Copy link
Contributor

This looks great jacobly0! Could you please put an #ifdef Q_OS_LINUX around this so it does not impact Win/Mac builds?

We have a 2.8 release in testing right now, but as soon as that is up I'd like to merge this for our next release!

@JonathonH-PIA
Copy link
Contributor

This needs a #include <QStandardPaths> too to build with Qt 5.15.2 but I'll add it after merging.

@jacobly0 Let me know if you'd like a credit in the next changelog and whether you'd prefer your username or real name! My email in in my profile if you'd rather get in touch that way.

@JonathonH-PIA JonathonH-PIA merged commit ecd5df5 into pia-foss:master Apr 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants