Support installing multiple Samba packages
ClosedPublic

Authored by ngraham on May 21 2019, 9:35 PM.

Details

Summary

Some distros (such as Manjaro) require multiple packages to be installed before a working
Samba stack is present. This patch adjusts the CMake packaging and Samba auto-
installation feature to support passing a comma-separated list of packages to support
such distos.

Test Plan
  • Remove samba and try auto-installation again; it still works
  • Invoke CMake with multiple packages (e.g. samba,rename) and try auto-installation again; both packages and their dependencies get installed

Diff Detail

Repository
R432 File Sharing (Samba) integration
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham requested review of this revision.May 21 2019, 9:35 PM
ngraham created this revision.
apol added a comment.May 21 2019, 11:33 PM

Hope this helps ^^'

samba/filepropertiesplugin/sambausershareplugin.cpp
149–165

This is a bit weird now, because you'll be getting N packages, and when they all finish installing they will all call "packageFinished" which I could expect to break in different ways.

It would look something like:

PackageKit::Transaction *transaction = PackageKit::Daemon::resolve(distroSambaPackages, PackageKit::Transaction::FilterArch);

QSharedPointer<QStringList> pkgids(new QStringList);
connect(transaction,
        &PackageKit::Transaction::package,
        [pkgids] (PackageKit::Transaction::Info info, const QString& packageId, const QString& summary) { pkgids.append(packageId); });
connect(t, &PackageKit::Transaction::finished, [pkgids] (PackageKit::Transaction::Exit exit) {
    if (exit != PackageKit::Transaction::ExitSuccess)
        return;
    auto t = PackageKit::Daemon::installPackage(*pkgids);
    //t....
})
bruns added a subscriber: bruns.May 22 2019, 12:47 PM

There are two executables required two run the samba integration properly:

  • smbd
  • net

But this is actually an implementation detail of KSambaShare from KIOCore. Wouldn't it be better to query the required packages from KSambaShare instead?

There are two executables required two run the samba integration properly:

  • smbd
  • net

    But this is actually an implementation detail of KSambaShare from KIOCore. Wouldn't it be better to query the required packages from KSambaShare instead?

Hmm, maybe. I don't see where that's actually specified in KSambaShare though. Can you help me find it?

ngraham updated this revision to Diff 58484.May 22 2019, 4:53 PM

Make it work (Thanks @apol for contributing virtually all of this code)

ngraham marked an inline comment as done.May 22 2019, 4:55 PM
apol accepted this revision.May 22 2019, 6:23 PM

I wouldn't have made it better :D

This revision is now accepted and ready to land.May 22 2019, 6:23 PM
This revision was automatically updated to reflect the committed changes.
bruns added inline comments.May 22 2019, 9:50 PM
samba/filepropertiesplugin/sambausershareplugin.cpp
43

see here ^

net is not checked for at all, it just tries to execute it using QProcess::start("net", args)