Re: [yast-devel] Sending Yast::FileUtils missing methods to the FileUtils Ruby module

On 12/5/19 1:11 PM, Josef Reidinger wrote:

V Thu, 5 Dec 2019 12:08:40 +0000 David Díaz 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/F... [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+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org