October 2011 - YaST Development - openSUSE Mailing Lists

[yast-devel] Re: [yast-commit] r66316 - in /trunk/snapper: VERSION agent-snapper/src/SnapperAgent.cc agent-snapper/src/SnapperAgent.h package/yast2-snapper.changes src/Snapper.ycp src/dialogs.ycp
by Ladislav Slezak 10 Oct '11

10 Oct '11

Dne 7.10.2011 11:16, jsuchome(a)svn2.opensuse.org napsal(a): > -string SnapperAgent::getValue (const YCPMap map, const string key) > +string SnapperAgent::getValue (const YCPMap map, const string key, string deflt) > { > if (!map->value(YCPString(key)).isNull() > && map->value(YCPString(key))->isString()) > return map->value(YCPString(key))->asString()->value(); > else > - return ""; > + return deflt; > } The "key" parameter is not changed so you could pass a reference to it ("const string &key"), that will prevent from creating unnecessary copy of the string on function call. "deflt" is also not changed, passing "const string &deflt" would help (I'm not sure, I'm not C++ expert, but then you might need to create the copy explicitly via return string(deflt); at the end.) > @@ -252,13 +282,10 @@ > /** > * Return the given file mode as octal number > */ > -global integer GetFileMode (string file) { > +integer GetFileMode (string file) { > > map out = (map) SCR::Execute (.target.bash_output, "/bin/stat --printf=%a " + file); When building custom shell commands String::Quote() should be always used, it prevents from errors when the input contains spaces and improves security (command injection) if the input comes from user. > if (Popup::AnyQuestionRichText (_("Restoring files"), > // popup message, %1 is snapshot number, %2 list of files > sformat (_("These files will be copied from snapshot '%1' to current system: %2Are you sure?"), > - previous_num, mergestring (files, " ")), > + previous_num, mergestring (to_restore, " ")), Similar problem, although very unlikely, if a file name contains an HTML tag it would be wrongly displayed. Use String::EscapeTags(). > - SCR::Execute (.target.bash, sformat ("/bin/cp -a '%1' '%2'", orig, file)); > + SCR::Execute (.target.bash, sformat ("/bin/cp -a '%1' '%2'", orig, full_path)); String::Quote() should be also used here, '%1' helps when there is a space, but doesn't help when the string contains ' (a single quote). And if the command supports (cp does) we should use -- option to stop the option processing. If %1 begins with -- it would be processed as an option, in some cases this can completely change the meaning of the command. The most fool proof way is to execute shell commands like this: SCR::Execute (.target.bash, sformat ("/bin/cp -a -- '%1' '%2'", String::Quote(orig), String::Quote(full_path))); (Note: These problems are not critical in YaST because YaST is started by root, but it's more critical in webyast and therefore we should get used to think about these possible problems.) -- Ladislav Slezák Appliance department / YaST Developer Lihovarská 1060/12 190 00 Prague 9 / Czech Republic tel: +420 284 028 960 lslezak(a)suse.com SUSE -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org To contact the owner, e-mail: yast-devel+owner(a)opensuse.org

3 2

[yast-devel] Re: [yast-commit] r66278 - in /branches/SuSE-Code-11-SP2-Branch/pos-installation: ./ package/ src/clients/ src/modules/
by Ladislav Slezak 07 Oct '11

07 Oct '11

Dne 6.10.2011 08:51, jsuchome(a)svn2.opensuse.org napsal(a): > + // help text > + string help_text = _("FIXME fill this"); Do not forget to fix it ;-) > + > + while (true) > + { > + ret = (symbol) UI::PollInput(); > + if (SCR::Read(.process.running, pid) != true) > + { > + update_output (); > + // explicitely check the process buffer after exit (bnc#488799) > + string buf = (string) SCR::Read (.process.read, pid); > + string err_buf = (string) SCR::Read (.process.read_stderr, pid); > + if (buf != nil && buf != "") > + UI::ChangeWidget (`id(`stdout), `LastLine, buf + "\n"); > + if (err_buf != nil && err_buf != "") > + UI::ChangeWidget (`id(`stderr), `LastLine, err_buf + "\n"); > + > + integer status = (integer) SCR::Read (.process.status, pid); > + y2internal ("exit status of the script: %1", status); > + UI::ReplaceWidget (`id (`rp_label), > + // text label > + `Label (`id (`label), `opt (`boldFont), _("Installation is completed.")) > + ); > + break; > + } > + else > + { > + update_output (); > + } > + if (ret == `cancel || ret == `abort) > + { > + SCR::Execute (.process.kill, pid, 15); > + UI::ReplaceWidget (`id (`rp_label), > + // text label > + `Label (`id (`label), `opt (`boldFont), _("Installation has been aborted.")) > + ); > + break; > + } > + sleep (100); > + } > + > + SCR::Execute (.process.kill, pid); It seems that the same (or very similar) code is also in firstboot_slepos_synchronization.ycp file, maybe it could be factored out and shared... > + while (true) > + { > + ret = UI::UserInput (); > + if (ret == `abort && !Popup::ConfirmAbort (`incomplete)) > + { > + continue; > + } > + else > + { > + break; > + } > + } I think this could be rewritten to something like this: do { ret = UI::UserInput(); } while(ret == `abort && !Popup::ConfirmAbort (`incomplete)); -- Ladislav Slezák Appliance department / YaST Developer Lihovarská 1060/12 190 00 Prague 9 / Czech Republic tel: +420 284 028 960 lslezak(a)suse.com SUSE -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org To contact the owner, e-mail: yast-devel+owner(a)opensuse.org

2 1

[yast-devel] PATCH: Clean up library to set dialog title and icon
by Stanislav Visnovsky 07 Oct '11

07 Oct '11

Hi! I wanted to start setting the dialog title for YaST dialogs and found out there is the infastructure prepared on the libyui-side. Just YCP does not use it. This patch improves the Wizard library a bit to do setting both icon and dialog title for the wizard dialog - the values are taken from the .desktop file and are localized. All you need to do in a module now is to replace Wizard::SetDesktopIcon("desktop-file-name"); to Wizard::SetDesktopTitleAndIcon("desktop-file-name"); If you accept the patch, I will go through the modules and start to change them to use the new functionality. Comments welcome! Stano P.S. There is no testsuite at the moment - desktop file reading involves agents (see Lukas' mail about trouble there), also Wizard testsuite is rather non-existent.

4 10

[yast-devel] YaST Code Reviews
by Thomas Goettlicher 05 Oct '11

05 Oct '11

Hey developers, Code reviews examine source code changes in order to find mistakes and increase code quality. YaST code can also profit from systematic code reviews. We should agree on a general way how to do code reviews for changes of YaST code in the future. From my perspective - code reviews should be optional - everyone should be permitted to read and review foreign code - everyone should be able to ask for a code review for specific changes - if you change somebody else's code you should ask for a review - nobody should be forced to have his changes reviewed if that causes a backlog (e.g. potential reviewer is busy or on vacation) What's your opinion? Cheers, Thomas -- Thomas Goettlicher SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg) Maxfeldstraße 5 90409 Nürnberg Germany -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org To contact the owner, e-mail: yast-devel+owner(a)opensuse.org

2 1

[yast-devel] yast2-devtools: all-packages
by Stanislav Visnovsky 05 Oct '11

05 Oct '11

Hi! I've just removed all-packages list from yast2-devtools. Also, the check for the file during 'make package' is not there anymore. If you want to live in the fantastic world without editting 'all-packages' file, make sure to make -f Makefile.cvs after installing yast2-devtools 2.21.3 Stano -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

3 5

[yast-devel] Re: [yast-commit] r66101 - /trunk/sound/sound/src/Joystick.ycp
by Stanislav Visnovsky 05 Oct '11

05 Oct '11

On Tuesday 27 of September 2011 16:44:37 lslezak(a)svn2.opensuse.org wrote: > Author: lslezak > Date: Tue Sep 27 16:44:36 2011 > New Revision: 66101 > > URL: http://svn.opensuse.org/viewcvs/yast?rev=66101&view=rev > Log: > - use Service::Start()/Stop() instead of hardcoded scripts (systemd > support) > > Modified: > trunk/sound/sound/src/Joystick.ycp > > Modified: trunk/sound/sound/src/Joystick.ycp > URL: > http://svn.opensuse.org/viewcvs/yast/trunk/sound/sound/src/Joystick.ycp?re > v=66101&r1=66100&r2=66101&view=diff > ========================================================================== > ==== --- trunk/sound/sound/src/Joystick.ycp (original) > +++ trunk/sound/sound/src/Joystick.ycp Tue Sep 27 16:44:36 2011 > @@ -251,8 +251,7 @@ > * Stop joystick service > */ > global define boolean Stop() ``{ Would be nice to also remove 'define' and backticks, when you actually touch the code. Stano > - return > - (SCR::Execute(.target.bash, "/etc/init.d/joystick stop", $[]) == 0); > + return Service::Stop("joystick"); > } > > /** > @@ -268,7 +267,8 @@ > global define boolean StartAndEnable() ``{ > > if (start == "enable") > - SCR::Execute(.target.bash, "/etc/init.d/joystick start", $[]); > + Service::Start("joystick"); > + > Service::Adjust("joystick", start); > return true; > } -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

3 2

[yast-devel] Pkg-bindings cleanup - summary of removed obsoleted functions
by Ladislav Slezak 04 Oct '11

04 Oct '11

Hi all, the weekend before Hackweek I did some cleanup in pkg-bindings, there were many obsoleted and unused functions (some even from prehistoric pre-libzypp time). * Note: the changes were done in Trunk only (for Factory/12.1), SP2 is unchanged! * I have adapted (or tried to adapt) all *.ycp files in trunk, so actually you do not need to do anything. But I could have overlooked something, just ask me if you find an error in your code related to pkg-bindings. Here are the details of the removed functions, they usually just returned bogus (empty) data: - selection related functions: Pkg::GetSelections(), Pkg::GetPatterns(), Pkg::SelectionData(), Pkg::SelectionContent(), Pkg::SetSelection(), Pkg::ClearSelection(), Pkg::ActivateSelections() (selections were replaced by patterns) - Pkg::TargetLogfile(), Pkg::LastErrorId(), Pkg::Init(), Pkg::SourceSetRamCache(), Pkg::InstSysMode(), Pkg::SourceProduct(), Pkg::SourceCleanupBroken(), Pkg::SourceGetBrokenSources(), Pkg::SourceFinish(), Pkg::SourceSaveRanks(), Pkg::SourceInstallOrder(), Pkg::PkgFreshen(), Pkg::PkgEstablish(), Pkg::CallbackAcceptNonTrustedGpgKey() (not supported/needed in libzypp anymore) - Pkg::SourceProvideDir() - obsoleted by SourceProvideDirectory() which has more options - Pkg::PkgAnyToDelete(), Pkg::PkgAnyToInstall() - obsoleted by IsAnyResolvable(), works also with patterns, patches... - locale functions: Pkg::GetLocale(), Pkg::SetLocale() (obsoleted by Get/SetTextLocale() and Get/SetPackageLocale() to set libzypp text output and packages to install independently) - YOU related functions and callbacks: Pkg::You*(), Pkg::CallbackYou*() (update repos are treated the same way as standard repos, SLES8/9 history) - Pkg::PatternData(), Pkg::TargetProducts() - obsoleted by Pkg::ResolvableProperties() -- Best Regards Ladislav Slezák Yast Developer ------------------------------------------------------------------------ SUSE LINUX, s.r.o. e-mail: lslezak(a)suse.cz Lihovarská 1060/12 tel: +420 284 028 960 190 00 Prague 9 fax: +420 284 028 951 Czech Republic http://www.suse.cz/ -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

1 0

[yast-devel] Webyast - plugin location
by Josef Reidinger 04 Oct '11

04 Oct '11

Hi, now in git we have placed various plugin under separate directory and not under vendor/plugin. This situation leads to various hacks with load path and non-working tools for rails. I don't see reason for it ( packaging we can solve more elegant, then forcing separate directories ), so I suggest to move it to plugin directory. Any opinions? Josef -- Josef Reidinger Software Engineer Appliance Department SUSE LINUX, s. r. o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic jreidinger(a)suse.com SUSE -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

2 1

[yast-devel] Re: [yast-commit] r66181 - /trunk/yast2/package/yast2.changes
by Ladislav Slezak 03 Oct '11

03 Oct '11

Hi all, please double check your email in *.changes when adding a new entry via 'osc vc', esp. on a machine outside of suse network. It also has happened to me, maybe we should add some check to refuse @site emails... Dne 29.9.2011 17:05, lslezak(a)svn2.opensuse.org napsal(a): > Author: lslezak > Date: Thu Sep 29 17:05:28 2011 > New Revision: 66181 > > URL: http://svn.opensuse.org/viewcvs/yast?rev=66181&view=rev > Log: > yast2.changes - fixed email address > > Modified: > trunk/yast2/package/yast2.changes > > Modified: trunk/yast2/package/yast2.changes > URL: http://svn.opensuse.org/viewcvs/yast/trunk/yast2/package/yast2.changes?rev=… > ============================================================================== > --- trunk/yast2/package/yast2.changes (original) > +++ trunk/yast2/package/yast2.changes Thu Sep 29 17:05:28 2011 > @@ -1,5 +1,5 @@ > ------------------------------------------------------------------- > -Mon Sep 26 10:58:19 UTC 2011 - jsrain@site > +Mon Sep 26 10:58:19 UTC 2011 - jsrain(a)suse.cz > > - simplify usage of save_y2logs (bnc#673990) > > -- -- Ladislav Slezák Appliance department / YaST Developer Lihovarská 1060/12 190 00 Prague 9 / Czech Republic tel: +420 284 028 960 lslezak(a)suse.com SUSE -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org For additional commands, e-mail: yast-devel+help(a)opensuse.org

2 1