On 12/5/19 1:11 PM, Josef Reidinger wrote:
V Thu, 5 Dec 2019 12:08:40 +0000
David Díaz <dgonza...@suse.de> napsáno:
Hi folks!
Yesterday, we got a bug report about a crash at the moment to save the
yast2-ftp-server configuration when "Allowing for anonymous upload"
...
Caller: /usr/share/YaST2/modules/FtpServer.rb:522:in 'WriteUpload'
Details: undefined method 'mkdir' for
#<Yast::FileUtilsClass:0x00001002ce2a2c0>
...
After realizing that, probably, the code was written with the FileUtils
Ruby module[1] in mind but the Yast::FileUtils module[2] is being called
instead, two questions came two my mind
- How much time has been this broken without nobody noticing/reporting it?
The change was introduced exactly a year ago[3], do it means that the
yast2-ftp-server is no longer relevant?
Well, I use that module sometimes, but usually to just setup quickly ftp
server, so not so often.
- Do we have this problem in more places? I.e. Yast::FileUtils taking
the place of Object::FileUtils.
Yes, we had it in past, e.g. String versus Yast::String is also problem and
some others. My common recommendation is to always use `::` prefix when you
want to use ruby functionality and avoid collision with YaST namespace.
Other long term solution on which we are working is to have code under
'src/lib` that does not live in YaST namespace, so for YaST specific calls like
Yast::FileUtils you have to use full path.
I don't have the answer for any of them ;) but I have a proposal[4] for
the last one: to implement the `method_missing` in the Yast::FileUtils
and fallback to Object::FileUtils. That way, we could avoid similar crashes.
Well, it helps in this one specific case. I agree it is a bit robust, but can
lead to another bugs like If you want to use FileUtils.Exist and use instead
FileUtils.exists? which is very similar but later does not respect changed SCR
target. and some other methods that just differs in case ( which was already
bad for old YCP code which has various mixtures ). Also it does not help for
other modules.
Also I am not so big fan of this magic as it can often lead to WTF when you
read code and wonder how it can work when you see that it calls wrong object.
But I am also not strong against as I see some value in it.
Josef
What do you think?
[1] https://ruby-doc.org/stdlib-2.6.5/libdoc/fileutils/rdoc/FileUtils.html
[2]
https://github.com/yast/yast-yast2/blob/master/library/general/src/modules/FileUtils.rb
[3] https://github.com/yast/yast-ftp-server/pull/54
[4] https://github.com/yast/yast-yast2/pull/997
I would also avoid to use meta-programming magic like that. Sooner than
later it will bite your ass ;). Moreover (and even though it is not
published in any official place yet) we have some namespace conventions
for the new code [1], for example Y2Storage, Y2ServicesManager,
Y2Firewall, Y2Network, etc. So the new code will not live under Yast
namespace anymore and it will not be directly affected by this problem.
For older code, I would follow the Josef's suggestion: to use the scope
resolution operator to always start from the global scope (e.g.,
::FileUtils).
[1] https://gist.github.com/imobachgs/d90940315bd0471ee00b2a68ba22fbe5
--
José Iván López González
YaST Team at SUSE LINUX GmbH
IRC: jilopez
--
To unsubscribe, e-mail: yast-devel+unsubscr...@opensuse.org
To contact the owner, e-mail: yast-devel+ow...@opensuse.org
