-
Notifications
You must be signed in to change notification settings - Fork 574
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
cmd/snap: implement userd command as replacement for snapd-xdg-open #3260
Changes from all commits
46a9432
169b142
f4e173c
b8c31e3
3f8f542
c41ebcf
5bbb9bb
b0babff
8109a38
d7f1aea
41a5709
59ccf07
a7de2c2
6216329
5381db5
9684c11
adf835a
0bf51cb
bd5e6ad
9d91bf2
cb2a694
39d0a68
2d410ca
1c9e37e
1ba7608
f6825ad
304fe24
2e7c3db
5130d8f
a7615bb
7d851a5
8a52fef
ffabd40
650e6be
51f914d
2794891
30a3473
db2a2ee
905aec0
2bafc31
4946fc0
bccd1c8
36e3cc5
e2145db
e193577
48bae99
4befed2
57d1ce0
5f61d01
a2e0f1d
6364b02
8a3a84d
91303a9
aa65ec5
ef810c3
f649d89
a7ff87c
94d37ab
dcdeb10
475ff96
d497289
7e9f696
0c23762
c8d73d9
1f3049a
eabbd00
e726d73
c37d23a
528dd75
65625d9
3ab33bc
bb185e7
fcb0890
d740c70
d5a4b1d
bfcb309
804dcbb
5c2884b
cdbe78e
1782192
0ac7b57
3e04c6d
cf2ac03
278ac67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
// -*- Mode: Go; indent-tabs-mode: t -*- | ||
|
||
/* | ||
* Copyright (C) 2017 Canonical Ltd | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU General Public License version 3 as | ||
* published by the Free Software Foundation. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
* | ||
*/ | ||
|
||
package main | ||
|
||
import ( | ||
"fmt" | ||
"os" | ||
"os/signal" | ||
"syscall" | ||
|
||
"github.com/jessevdk/go-flags" | ||
|
||
"github.com/snapcore/snapd/i18n" | ||
"github.com/snapcore/snapd/userd" | ||
) | ||
|
||
type cmdUserd struct { | ||
userd userd.Userd | ||
} | ||
|
||
var shortUserdHelp = i18n.G("Start the userd service") | ||
var longUserdHelp = i18n.G("The userd command starts the snap user session service.") | ||
|
||
func init() { | ||
cmd := addCommand("userd", | ||
shortAbortHelp, | ||
longAbortHelp, | ||
func() flags.Commander { | ||
return &cmdUserd{} | ||
}, | ||
nil, | ||
[]argDesc{}, | ||
) | ||
cmd.hidden = true | ||
} | ||
|
||
func (x *cmdUserd) Execute(args []string) error { | ||
if len(args) > 0 { | ||
return ErrExtraArgs | ||
} | ||
|
||
if err := x.userd.Init(); err != nil { | ||
return err | ||
} | ||
x.userd.Start() | ||
|
||
ch := make(chan os.Signal) | ||
signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM, syscall.SIGUSR1) | ||
select { | ||
case sig := <-ch: | ||
fmt.Fprintf(Stdout, "Exiting on %s.\n", sig) | ||
case <-x.userd.Dying(): | ||
// something called Stop() | ||
} | ||
|
||
return x.userd.Stop() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
// -*- Mode: Go; indent-tabs-mode: t -*- | ||
|
||
/* | ||
* Copyright (C) 2016 Canonical Ltd | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU General Public License version 3 as | ||
* published by the Free Software Foundation. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
* | ||
*/ | ||
|
||
package main_test | ||
|
||
import ( | ||
"os" | ||
"syscall" | ||
"time" | ||
|
||
. "gopkg.in/check.v1" | ||
|
||
snap "github.com/snapcore/snapd/cmd/snap" | ||
"github.com/snapcore/snapd/testutil" | ||
) | ||
|
||
type userdSuite struct { | ||
BaseSnapSuite | ||
testutil.DBusTest | ||
} | ||
|
||
var _ = Suite(&userdSuite{}) | ||
|
||
func (s *userdSuite) TestUserdBadCommandline(c *C) { | ||
_, err := snap.Parser().ParseArgs([]string{"userd", "extra-arg"}) | ||
c.Assert(err, ErrorMatches, "too many arguments for command") | ||
} | ||
|
||
func (s *userdSuite) TestUserd(c *C) { | ||
go func() { | ||
defer func() { | ||
me, err := os.FindProcess(os.Getpid()) | ||
c.Assert(err, IsNil) | ||
me.Signal(syscall.SIGUSR1) | ||
}() | ||
|
||
needle := "io.snapcraft.Launcher" | ||
for i := 0; i < 10; i++ { | ||
for _, objName := range s.SessionBus.Names() { | ||
if objName == needle { | ||
return | ||
} | ||
time.Sleep(1 * time.Second) | ||
} | ||
|
||
} | ||
c.Fatalf("%s does not appeared on the bus", needle) | ||
}() | ||
|
||
rest, err := snap.Parser().ParseArgs([]string{"userd"}) | ||
c.Assert(err, IsNil) | ||
c.Check(rest, DeepEquals, []string{}) | ||
c.Check(s.Stdout(), Equals, "Exiting on user defined signal 1.\n") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
all install clean: | ||
$(MAKE) -C systemd $@ | ||
$(MAKE) -C dbus $@ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
# | ||
# Copyright (C) 2017 Canonical Ltd | ||
# | ||
# This program is free software: you can redistribute it and/or modify | ||
# it under the terms of the GNU General Public License version 3 as | ||
# published by the Free Software Foundation. | ||
# | ||
# This program is distributed in the hope that it will be useful, | ||
# but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
# GNU General Public License for more details. | ||
# | ||
# You should have received a copy of the GNU General Public License | ||
# along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
|
||
BINDIR := /usr/bin | ||
DBUSSERVICESDIR := /usr/share/dbus-1/services | ||
|
||
SERVICES_GENERATED := $(patsubst %.service.in,%.service,$(wildcard *.service.in)) | ||
SERVICES := ${SERVICES_GENERATED} | ||
|
||
%.service: %.service.in | ||
cat $< | sed 's:@bindir@:${BINDIR}:g' | cat > $@ | ||
|
||
all: ${SERVICES} | ||
|
||
install: ${SERVICES} | ||
install -D -m 0644 -t ${DESTDIR}/${DBUSSERVICESDIR} $^ | ||
|
||
clean: | ||
rm -f ${SERVICES_GENERATED} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
[D-BUS Service] | ||
Name=io.snapcraft.Launcher | ||
Exec=@bindir@/snap userd | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feels like libexecdir thing to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the regular There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is correct, though I expected this to be implemented in a libexecdir command, as users shouldn't usually do things here. Not a biggie, though. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,12 +86,23 @@ const unity7ConnectedPlugAppArmor = ` | |
/usr/bin/xdg-open ixr, | ||
/usr/share/applications/{,*} r, | ||
/usr/bin/dbus-send ixr, | ||
|
||
# This allow access to the first version of the snapd-xdg-open | ||
# version which was shipped outside of snapd | ||
dbus (send) | ||
bus=session | ||
path=/ | ||
interface=com.canonical.SafeLauncher | ||
member=OpenURL | ||
peer=(label=unconfined), | ||
# ... and this allows access to the new xdg-open service which | ||
# is now part of snapd itself. | ||
dbus (send) | ||
bus=session | ||
path=/io/snapcraft/Launcher | ||
interface=io.snapcraft.Launcher | ||
member=OpenURL | ||
peer=(label=unconfined), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 🎉 |
||
|
||
# input methods (ibus) | ||
# subset of ibus abstraction | ||
|
@@ -303,6 +314,11 @@ dbus (send) | |
bus=session | ||
interface=com.canonical.SafeLauncher.OpenURL | ||
peer=(label=unconfined), | ||
# new url helper (part of snap userd) | ||
dbus (send) | ||
bus=session | ||
interface=io.snapcraft.Launcher.OpenURL | ||
peer=(label=unconfined), | ||
|
||
# dbusmenu | ||
dbus (send) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ Build-Depends: autoconf, | |
autotools-dev, | ||
bash-completion, | ||
debhelper (>= 9), | ||
dbus, | ||
dh-apparmor, | ||
dh-autoreconf, | ||
dh-golang (>=1.7), | ||
|
@@ -64,8 +65,8 @@ Depends: adduser, | |
systemd, | ||
${misc:Depends}, | ||
${shlibs:Depends} | ||
Replaces: ubuntu-snappy (<< 1.9), ubuntu-snappy-cli (<< 1.9), snap-confine (<< 2.23), ubuntu-core-launcher (<< 2.22) | ||
Breaks: ubuntu-snappy (<< 1.9), ubuntu-snappy-cli (<< 1.9), snap-confine (<< 2.23), ubuntu-core-launcher (<< 2.22) | ||
Replaces: ubuntu-snappy (<< 1.9), ubuntu-snappy-cli (<< 1.9), snap-confine (<< 2.23), ubuntu-core-launcher (<< 2.22), snapd-xdg-open (<< 0.0.0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Finally, thank you again for doing this :-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is an insane number of |
||
Breaks: ubuntu-snappy (<< 1.9), ubuntu-snappy-cli (<< 1.9), snap-confine (<< 2.23), ubuntu-core-launcher (<< 2.22), snapd-xdg-open (<< 0.0.0) | ||
Conflicts: snap (<< 2013-11-29-1ubuntu1) | ||
Built-Using: ${Built-Using} ${misc:Built-Using} | ||
Description: Tool to interact with Ubuntu Core Snappy. | ||
|
@@ -112,3 +113,11 @@ Section: oldlibs | |
Pre-Depends: dpkg (>= 1.15.7.2) | ||
Description: Transitional package for snapd | ||
This is a transitional dummy package. It can safely be removed. | ||
|
||
Package: snapd-xdg-open | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. keeping this as separate package will break for users using only "apt-get upgrade" (vs dist-upgrade) if snapd gets a dependency on it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similar to snap-confine btw ... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It will stay a separate package but produced by the snapd source package so we can migrate people away from the other snapd-xdg-open deb package. That was the idea. Didn't tested yet this works well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well, but that means you have to either add a recommends (in which case the package will never be installed for users only doing "apt-get upgrade") or you add a hard dependency in which case users of "apt-get upgrade" will get an upgrade error. we have tons of bugs for that case from snap-confine which was one of the reasons to include it without a separate package, keeping a separate binary package looks to me like we are exactly re-introducing the same problem again just with another package now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need one or another way to get the snapd-xdg-open package removed from installations out there. If we have a binary package produced by two different source packages but the second one has a higher version as the first one isn't then the one from the first source package overwritten? The Recommends is for sure missing. Any other ways how we can achieve this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, include the binaries in snapd and have the proper breaks/replaces lines (and perhaps a provides) in the snapd package. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, including the binaries in the snapd package was the plan. Will add the Breaks/Replaces line. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CC @mvo5 - I think we just want to have this in the main snapd package and have snapd-xdg-open as an empty transitional package. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
Architecture: any | ||
Depends: snapd (= ${binary:Version}), ${misc:Depends} | ||
Section: oldlibs | ||
Pre-Depends: dpkg (>= 1.15.7.2) | ||
Description: Transitional package for snapd-xdg-open | ||
This is a transitional dummy package. It can safely be removed. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -119,8 +119,8 @@ override_dh_auto_build: | |
cd cmd && ( ./configure --prefix=/usr --libexecdir=/usr/lib/snapd $(VENDOR_ARGS)) | ||
$(MAKE) -C cmd all | ||
|
||
# Generate the real systemd units out of the available templates | ||
$(MAKE) -C data/systemd all | ||
# Generate the real systemd/dbus config files | ||
$(MAKE) -C data all | ||
|
||
override_dh_auto_test: | ||
dh_auto_test -- $(GCCGOFLAGS) | ||
|
@@ -210,10 +210,11 @@ override_dh_install: | |
cp -R share/locale debian/snapd/usr/share; \ | ||
fi | ||
|
||
# install snapd's systemd units, done here instead of | ||
# debian/snapd.install because the ubuntu/14.04 release | ||
# branch adds/changes bits here | ||
$(MAKE) -C data/systemd install DESTDIR=$(CURDIR)/debian/snapd/ SYSTEMDSYSTEMUNITDIR=$(SYSTEMD_UNITS_DESTDIR) | ||
# install snapd's systemd units / upstart jobs, done | ||
# here instead of debian/snapd.install because the | ||
# ubuntu/14.04 release branch adds/changes bits here | ||
$(MAKE) -C data install DESTDIR=$(CURDIR)/debian/snapd/ \ | ||
SYSTEMDSYSTEMUNITDIR=$(SYSTEMD_UNITS_DESTDIR) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wish we had one spelling for this variable. |
||
|
||
$(MAKE) -C cmd install DESTDIR=$(CURDIR)/debian/tmp | ||
|
||
|
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.
Thank you for using
$(MAKE)