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

Fix pledge() usage and make test cases work properly on OpenBSD #862

Closed
wants to merge 3 commits into from

Conversation

cgull
Copy link
Member

@cgull cgull commented Mar 20, 2017

Tested on OpenBSD 6.0.

This might or might not make it into mosh 1.3.0, but if not will be committed immediately after release. I'm somewhat inclined to do this after release. Jeremie, would it be possible for you to do this in the OpenBSD port for this release? @keithw, do you have any opinion one way or another?

jcourreges and others added 3 commits March 20, 2017 09:54
Unbreaks mosh on recent (> 2017/03/18) OpenBSD systems.
This problem was seen on OpenBSD 6.0.
This tests for OpenBSD 6.x or later.  We could likely extend the
test to much older OpenBSD versions, but not without OS installs
and testing.
@keithw
Copy link
Member

keithw commented Mar 20, 2017

@cgull, what are the implications if we release 1.3.0 without this fix? Does Mosh still work on OpenBSD...? I'm a little unclear on what's broken in the current master.

@cgull
Copy link
Member Author

cgull commented Mar 20, 2017

  • A couple of weeks ago, Mosh built and ran on OpenBSD 6.0 and probably prior releases. But as is, it would not run test cases that require tmux, because tmux on OpenBSD is not the portable version and does not have the -V flag to report a version number (I'd been working around it with a local build of the portable version of tmux).
  • A week or so ago, I added code that tries to ensure a UTF-8 locale is available during tests. That errors tests that use it on OpenBSD (and probably some other systems) because its locale command does not support the charmap argument (which is an extension beyond POSIX functionality).
  • 3/18/2017, OpenBSD altered pledge(2) in a backwards incompatible way, dropping support for one of its possible arguments. That change will be in OpenBSD 6.1 releasing in a couple of weeks.
  • Jeremie worked up a patch to fix that problem.
  • I tested this with make check on OpenBSD 6.0 and saw the locales problem, and also realized I could fix the tmux problem. I only did this back to 6.0 because that's what I have installed and 5.9 goes out of support when 6.1 is released.

So, before these changes, mosh would build on any OpenBSD (but make check would fail), but would fail at startup on OpenBSD-current and the coming OpenBSD 6.1. After these changes, make, make check, and Mosh itself will run correctly and completely on OpenBSD 6.0-6.1. They should also run correctly on earlier versions of OpenBSD, but without running the tmux-based tests.

@cgull
Copy link
Member Author

cgull commented Mar 20, 2017

For testing the locale, I've realized that the better thing to do is write a wrapper program for our is_utf8_locale() function, which gets us the same test that Mosh does on all platforms. That's probably not for 1.3.0, though.

@keithw
Copy link
Member

keithw commented Mar 21, 2017

@cgull Honestly I'm inclined just to go with your recommendation on this one, but if you want my two cents, it seems like we may as well include this in 1.3.0 and credit our thorough release candidate process for catching this. :-) I'd rather that Mosh 1.3.0 work out-of-the-box on OpenBSD since they were kind enough to test our RC and report a bug.

@cgull
Copy link
Member Author

cgull commented Mar 21, 2017

Hmm. The pledge() change poses no risk to platforms other than OpenBSD, it's under #ifdef PLEDGE. The other changes do pose regression risk for make check on other platforms. So I think I will pull Jérémie's fix for that, and work on the other changes for post-release.

@cgull cgull mentioned this pull request Apr 9, 2017
@cgull
Copy link
Member Author

cgull commented Apr 25, 2017

This is all pulled now.

@cgull cgull closed this Apr 25, 2017
@cgull cgull deleted the openbsd-pledge-tmux-fixes branch August 1, 2017 23:17
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

3 participants