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

sshserver.pl fixes #715

Closed
wants to merge 2 commits into from
Closed

Conversation

Karlson2k
Copy link
Contributor

Testsuite running SSH script fixes.

At least on Msys2, perl fails to redirect stderr if $value contains
newline or other weird characters.
@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @yangtse, @mback2k and @dfandrich to be potential reviewers

@bagder
Copy link
Member

bagder commented Mar 16, 2016

AuthorizedKeysFile2 was removed in OpenSSH 5.9, released in September 2011. I say we just stop using that option completely everywhere. Agree?

@Karlson2k
Copy link
Contributor Author

@bagder Agree.
Update this PR?

@bagder bagder closed this in 2716350 Mar 20, 2016
@captain-caveman2k
Copy link
Contributor

I don't know if this is related but a bunch of autobuilds seem to be failing, as of this commit, due to issues starting the SSH server:

  • Dan Fandrich's torture tests
  • My Ubuntu 14.04 and 15.04 builds but not my 15.10 builds

I haven't looked at the problem in detail but just wanted to make you aware of the problem before release.

@bagder
Copy link
Member

bagder commented Mar 22, 2016

Thanks! Hm. A quick google tells me ubuntu 14.04 has a newer openssh server than 5.9 and thus support for that config option was already removed then.

But ok, let me revert that for now and see if we get back some working builds.

@bagder
Copy link
Member

bagder commented Mar 22, 2016

commit d5e7f50 reverted the commit

@bagder bagder reopened this Mar 22, 2016
@Karlson2k
Copy link
Contributor Author

@bagder This PR shouldn't break anything.
However, strange that removing usage of undocumented option breaks something.

@bagder
Copy link
Member

bagder commented Mar 22, 2016

I agree, but since something broke I'm testing it out by reverting this...

@captain-caveman2k
Copy link
Contributor

Ironically, with fantastic timing, it looks like my autobuild VMs have gone down - my email server is still up so I'm not sure what is wrong at the moment :(

Hopefully Dan's tests will run over night.

@captain-caveman2k
Copy link
Contributor

It turns out I had filled up my mailbox on my mail server - deleting some of the emails has enabled my autobuild emails to go out now.

As such, you can see that the 14:30 Ubuntu 14.04 and 16:30 15:04 are now passing as they did previously. I don't know why this fix affected them as it did - but I will try and investigate at the weekend.

Unfortunately I only have 15.10 on my laptop which wasn't affected by the change.

@bagder
Copy link
Member

bagder commented Mar 22, 2016

Thanks @captain-caveman2k . So @Karlson2k, what was your original reason for removing this option? Was it something that failed to work properly with it still present?

@Karlson2k
Copy link
Contributor Author

@bagder No, it was just annoying strings in logs.
Pay attention to second commit. It helps not only with stderr, but also with any parameter with space or tab.

@bagder
Copy link
Member

bagder commented Mar 26, 2016

Pay attention to second commit. It helps not only with stderr, but also with any parameter with space or tab.

And we have a problem with that?

@bagder
Copy link
Member

bagder commented Mar 26, 2016

That quote fix is now in commit e326448. If I may ask, please read https://curl.haxx.se/dev/contribute.html#Write_good_commit_messages for a small "guideline" on how we prefer our commit messages. It would make me merge your fixes even faster!

@Karlson2k
Copy link
Contributor Author

Pay attention to second commit. It helps not only with stderr, but also with any parameter with space or tab.

And we have a problem with that?

If any path is passed in parameter - it may be misinterpreted as several parameters as path may contain spaces.

@Karlson2k
Copy link
Contributor Author

If I may ask, please read https://curl.haxx.se/dev/contribute.html#Write_good_commit_messages for a small "guideline" on how we prefer our commit messages.

I read it already, but it isn't easy to keep all different preferences for different project in my mind.
Will try better! 😄

@bagder
Copy link
Member

bagder commented Mar 31, 2016

As that old config option doesn't seem to harm anyone but caused trouble when we tried to remove it, let's leave it there a few more years and then do another attempt! case closed for this time.

@bagder bagder closed this Mar 31, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants