-
Notifications
You must be signed in to change notification settings - Fork 5
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
Pure ftp drop #27
Pure ftp drop #27
Conversation
package/yast2-ftp-server.changes
Outdated
- convert starting on demand from Xinetd to systemd sockets due | ||
to drop of Xinetd (fate#323373) | ||
- Drop pure-ftp as it does not provide systemd socket and we want | ||
to support only one backend to reduce maintenance loadout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maintenance loadout
=> the maintenance charge
Release: 0 | ||
|
||
BuildRoot: %{_tmppath}/%{name}-%{version}-build | ||
Source0: %{name}-%{version}.tar.bz2 | ||
|
||
# SuSEFirewall2 replace by firewalld (fate#323460) | ||
Requires: yast2 >= 4.0.39 | ||
Requires: yast2-inetd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we only support one backend pure-ftpd
can be removed from the description.
@@ -355,25 +313,25 @@ def main | |||
"Anonymous users can create directories." | |||
) | |||
}, | |||
#only vsftd | |||
# only vsftd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NP: We were removing allusions to only vsftpd
below, so why not just remove them completely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense
src/clients/ftp-server.rb
Outdated
CommandLine.Print("") | ||
# TRANSLATORS: CommandLine header | ||
CommandLine.Print(String.UnderlinedHeader(_("Start-Up:"), 0)) | ||
CommandLine.Print("") | ||
# TRANSLATORS: CommandLine progress information | ||
CommandLine.Print(_("Start FTP daemon via xinetd")) | ||
CommandLine.Print(_("Start FTP daemon via socket")) | ||
CommandLine.Print("") | ||
SetStartedViaXinetd(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this call be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why it should be removed? It is CLI that inform you that FTP Daemon is starting by socket listening.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean the SetStartedViaXinetd(true)
at least should be renamed as commented before trying to remove allusions to xinetd, so rename to a name that correspond with the current behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, make sense, now it do socket enablement.
src/clients/ftp-server.rb
Outdated
CommandLine.Print("") | ||
CommandLine.Print(String.UnderlinedHeader(_("Display Settings:"), 0)) | ||
CommandLine.Print("") | ||
#start-up settings | ||
# start-up settings | ||
CommandLine.PrintNoCR(_("Start-Up:")) | ||
if GetStartedViaXinetd() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this call be removed as commented below with Set method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why it should be removed? When systemd socket start ftp daemon, then it should be written. There are three states: socket, service and manual and this is one of the states.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As with Set I would rename it to correspond the behavior with the naming
@@ -2,19 +2,10 @@ | |||
|
|||
require "yast" | |||
|
|||
require "y2firewall/firewalld" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thnx!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
https://trello.com/c/uL3kzh2z/1322-8-remove-yast-xinetd-autoyast-ftp-server-and-inetd-module
after approval another round of manual testing is planned. The first round was successful on SLE15.