[systemd-devel] sd-boot on Fedora 30?

[Filipe Brandenburger]

Hi,

I've been trying to get sd-boot to work on Fedora 30, made some progress
but not fully there yet...

First I found my partition GPT type in /boot was incorrect and bootctl was
trying to use /boot/efi instead. Ok, that fixed, now I get a list of
kernels.

But whenever I boot, I only get the "Reboot Into Firmware Interface" menu
entry and nothing else...

I imagine this might be related to the Grub entries:

$ sudo bootctl list
/boot/loader/entries/4d3fcddc096748c4a398037699515189-5.2.8-200.fc30.x86_64.conf:7:
Unknown line "id", ignoring.
/boot/loader/entries/4d3fcddc096748c4a398037699515189-5.2.8-200.fc30.x86_64.conf:8:
Unknown line "grub_users", ignoring.
/boot/loader/entries/4d3fcddc096748c4a398037699515189-5.2.8-200.fc30.x86_64.conf:9:
Unknown line "grub_arg", ignoring.
/boot/loader/entries/4d3fcddc096748c4a398037699515189-5.2.8-200.fc30.x86_64.conf:10:
Unknown line "grub_class", ignoring.
title: Fedora (5.2.8-200.fc30.x86_64) 30 (Workstation Edition)
(default)
   id: 4d3fcddc096748c4a398037699515189-5.2.8-200.fc30.x86_64
   source:
/boot/loader/entries/4d3fcddc096748c4a398037699515189-5.2.8-200.fc30.x86_64.conf
  version: 5.2.8-200.fc30.x86_64
linux: /vmlinuz-5.2.8-200.fc30.x86_64
   initrd: /initramfs-5.2.8-200.fc30.x86_64.img
  options: $kernelopts

I tried to at least fix the $kernelopts one, with grubby --args="..."
adding a dummy argument just to deduplicate it from the grubenv contents,
but still couldn't boot from there...

Even if I fix that, looks like new kernels installed would trigger
/usr/lib/kernel/install.d/20-grub.install and probably mess up that setup
(do I have to mask or remove it completely?)

Fedora's BLS document unfortunately doesn't mention sd-boot at all :-(
https://fedoraproject.org/wiki/Changes/BootLoaderSpecByDefault

Anyways, if anyone has hints of what I could try next, I'd be quite
interested to know. (Perhaps adding some docs to Fedora wiki would be
pretty helpful too!) I thought I'd ask here first... If I don't hear back,
I might try to ask on Fedora lists instead.

Cheers!
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Delegate= on slice before v237

[Filipe Brandenburger via systemd-devel]

Hey Lennart,

Thanks for the clarification.

On Tue, Feb 12, 2019 at 2:17 AM Lennart Poettering 
wrote:

> On Mo, 11.02.19 16:39, Filipe Brandenburger (filbran...@google.com) wrote:
> > Before systemd v237 (when Delegate= was no longer allowed on slice
> > units)... Did setting Delegate=yes on a slice have *any* effect at all?
> >
> > Or did it just do nothing (and a slice with Delegate=no or no setting
> > behave just the same)?
> >
> > Reason I ask is: I want to scrap this code
> > <
> https://github.com/opencontainers/runc/blob/v1.0.0-rc6/libcontainer/cgroups/systemd/apply_systemd.go#L195
> >
> > in libcontainer that tries to detect whether Delegate= is accepted in a
> > slice unit. (I'll just default it to false, never try it.)
> >
> > I'd like to be able to say that Delegate=yes never really did anything at
> > all on slice units... So I'm trying to confirm that is really the case
> > before stating it.
>
> So, it wasn't supposed to do anything, and what it does differs on
> cgroupsv2 and cgroupsv1.


libcontainer is pretty much cgroupv1 only, so that's what I'm concerned
about.


> The fact it wasn't refused outright was an
> accident, and because it was one I am not entirely sure what the
> precise effect of allowing it was. However, I am pretty sure it at
> least had two effects:
>
> 1. it would turn on all controllers for the cgroup
>

I don't *think* this is why libcontainer was trying to enable it, since a
few lines down it's explicitly enabling all the controllers by
setting MemoryAccounting, CPUAccounting and BlockIOAccounting during
transient unit creation:
https://github.com/opencontainers/runc/blob/v1.0.0-rc6/libcontainer/cgroups/systemd/apply_systemd.go#L275


> 2. it would stop systemd to ever migrating foreign processes below
>that slice, which is primarily relevant only when changing cgroup
>related props on the slice dynamically I guess.
>

I'm not sure I follow... Do you mean if libcontainer would write
to memory.limit_in_bytes (or one of the other properties of the memory or
other controller managed by systemd, such as cpu), then systemd would not
end up overwriting this as it does some other operation on the cgroup?

I'm not completely sure I understand what "migrate foreign processes"
means, given slices don't really hold any pids directly... Do you mean to
scope and service units below that slice?

In any case, for now I'll probably leave that alone... Though as I revamp
libcontainer support for unified hierarchy, I'll try to skip that check on
that case, that might make this a legacy-only setting, so not that
important to fully get rid of it for a while...

Cheers!
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] At wits end... need to execute a script prior to anything getting killed/changed on reboot/shutdown

[Filipe Brandenburger]

If you want to run it early in the shutdown process, then keep
DefaultDependencies=yes, in which case it will run before the base
dependencies start to get stopped.

If you need some other resources to be up, for instance network, then add
After=network.target, etc.

Remember that when shutting down, the dependencies are stopped in the
opposite order as they're started up, so if you need your script to run
*before* something else is stopped, then you need an After= dependency.

You shouldn't add any ordering regarding reboot.target and shutdown.target.
Just enable your service (so that it looks up during normal system usage),
when the system goes down it will be stopped, and then depending on its
After= dependencies it will block those other services from being stopped
until you're done.

In recent systemd versions (I think starting from v238?) you can omit the
ExecStart=/bin/true line, an unit without that line starts to be valid in
one of those versions... Though keeping it around is fine and will work
with older versions too.

I hope this helps!

Cheers,
Filipe


On Wed, Jan 16, 2019 at 10:47 AM Christopher Cox 
wrote:

> I need to be able to execute a script before anything gets shutdown.  That
> is,
> when somebody does a "reboot", "shutdown" or "poweroff", I need this
> script to
> run first, and for it to finish before everything gets whacked.
>
> I know the following isn't "right"... I've tried so many different
> things.
> Google hasn't helped only giving me many "right" solutions that didn't
> work. In
> this current edition, basically I get a partial capture of processes that
> are
> running (that is some were killed directly or indirectly).. I need them
> all.  My
> script needs to see the state of operation before reboot/shutdown/poweroff
> do
> anything else.  My "save" saves some information about running processes
> (some
> not necessarily under systemd control).
>
> [Unit]
> Description=my-service save status
> DefaultDependencies=no
> Before=reboot.target shutdown.target
> Conflicts=reboot.target shutdown.target
>
> [Service]
> Type=oneshot
> RemainAfterExit=yes
> ExecStart=/bin/true
> ExecStop=/usr/local/bin/my-service.sh save
> StandardOutput=journal
>
> [Install]
> WantedBy=multi-user.target
>
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Bugfix release(s)

[Filipe Brandenburger]

So I think all the bits already exist somewhere and perhaps a small
change in naming would go a long way to make these pushes smoother.

If when we cut v240 from the master branch, we had called it v240-rc1
instead, perhaps it was clear that it could take some more testing
before it was made official.

Furthermore, fixes for the breakage were backported into the
v240-stable tree in systemd-stable repository, so perhaps calling the
top of that tree v240 (or v240.0) at some point would have been
helpful.

Having been pushing to systemd-stable this week (fixing one of the
CVEs in previous versions), I have to say that there's some impedance
to contributing to that tree, since I needed a separate fork (GitHub
doesn't want to let me do PRs from my main fork), sometimes it doesn't
build on the latest toolchain and libs (I'm working on fixing that
too), etc. Perhaps having some more of the distro maintainers actively
helping on those branches would be best. I think bringing those
branches into the main repo would help in those regards.

Why don't we try something slightly different for the v241 timeline?

At the time of the release, we actually create a new *branch* and call
it release-v241. We also tag v241-rc1 at the start of that tree and
announce the pre-release. (Note that this branch means no need for
v241-stable in systemd-stable anymore, so it's not a branch which
wouldn't have existed, it's only in a different place now.)

As distros start to do heavier and broader testing of that
pre-release, we start fixing bugs at trunk, backport them to
release-v241 and after a week or so release v241-rc2. Rinse and
repeat.

After things are stable for a couple of weeks, we can finally just
bump the version number, tag v241.0 and announce the final release.
Hopefully everything will go very smooth. But, if it doesn't, we can
still iterate on that and release v241.1. We can also release v241.2
to address the CVEs that come up a month later (just kidding, of
course there won't be any!)

>From a developer's point of view, this really doesn't look too painful
compared with the current process.

And distros will have an useful "almost ready" point where they have
time to do one-time testing and start pushing to some users to collect
feedback before the final "official" or "stable" release.

What do you all think?

Cheers,
Filipe


smime.p7s
Description: S/MIME Cryptographic Signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] at replacement

[Filipe Brandenburger]

On Thu, Jul 12, 2018 at 2:06 PM Matt Zagrabelny  wrote:
> On Thu, Jul 12, 2018 at 3:07 PM, Filipe Brandenburger  
> wrote:
>> Take a look at systemd-run and, in particular, options such as
>> --on-active=, --on-calendar= and --timer-property=, which allow you to
>> set a .timer option on demand for a single one-off command.
>
> What sort of timer properties are folks setting or adjusting via the 
> --timer-property? That is, when would I use such an option?

Technically, any options accepted in systemd.timer are possible:
https://www.freedesktop.org/software/systemd/man/systemd.timer.html#Options

For instance, RandomizedDelaySec= might be an interesting one you
might want to use.

AccuracySec= is also one you potentially want to look into, if you
care about the accuracy of the time at which your command will run.

Cheers,
Filipe


smime.p7s
Description: S/MIME Cryptographic Signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] at replacement

[Filipe Brandenburger]

Hi,

On Thu, Jul 12, 2018 at 12:04 PM Matt Zagrabelny  wrote:
> I know systemd can replace cron. Do folks use it to replace "at", too?
>
> I know it *can* - with two files per "at" entry and then enabling and 
> starting the timer.
>
> Is there an easier with to replace "at" with systemd than creating two files 
> and enabling and starting the timer?

Take a look at systemd-run and, in particular, options such as
--on-active=, --on-calendar= and --timer-property=, which allow you to
set a .timer option on demand for a single one-off command.

https://www.freedesktop.org/software/systemd/man/systemd-run.html#--on-active=

I hope this helps!

Cheers,
Filipe


smime.p7s
Description: S/MIME Cryptographic Signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] When is a unit "loaded"?

[Filipe Brandenburger]

Hey Daniel!

On Wed, Jul 11, 2018 at 5:16 PM Daniel Wang  wrote:
> I have a unit, say foo.service, on my system that's in 
> /usr/lib/systemd/system, but disabled by preset.

Not that it matters, but presets don't really matter here. The unit is
disabled, period.

> On system boot, it doesn't show as "loaded" per `systemctl --all | grep foo`.

Because there's no reference to it in units systemd sees, so systemd
doesn't need to load it.

At bootup, systemd will simply start with default.target and then
recursively load the units it needs.

See this link for more details:
https://www.freedesktop.org/software/systemd/man/bootup.html#System%20Manager%20Bootup

`systemctl --all` will only show the units in memory, so foo.service
won't be listed since it's not loaded.

> So if I override it with a file with the same name but under 
> /etc/systemd/system, `systemctl cat foo.service` will show the one under /etc 
> without the need for a `systemctl daemon-reload`.

Yes, because it's not loaded.

> If I create another service unit, bar.service, which has a After= dependency 
> on foo.service, and start bar, foo.service will show as loaded. And then if I 
> try to override it, `systemctl cat foo.service` will print a warning saying a 
> daemon-reload is needed.

Yes. If systemd sees a reference for that unit (even if it's an
After=), it will need to load it, since it needs to record the
dependency between the units in the internal memory structures, so it
needs a reference to the unit, and it loads it to have a complete
reference to it...

> Nothings seems incorrect, but I have a few questions:
> - Which units are loaded on-boot and which are not?

Only default.target and recursively any unit referred to by the loaded units.

> - Is the After= dependency alone enough to have systemd load a unit? Are 
> there any other dependency directives that will result in the same effect?

Yes, I believe any reference to units will trigger the unit to be
loaded. As I mentioned, systemd wants to keep the state in memory, so
loading the unit will allow it to keep complete state.

An exception (haven't checked it, but I expect it) is references in
the [Install] section (such as Also=) since those are only used by
`systemctl enable` and are not really loaded into memory as far as I
can tell (but I might be wrong here, and these might trigger the unit
to load as well.)

I hope this helps!

Cheers,
Filipe


smime.p7s
Description: S/MIME Cryptographic Signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Apparmor in containers

[Filipe Brandenburger]

Hi,

Actually, it seems AppArmor has support for containers and can have a
specific profile for inside the containers only.

Docker does support it:
https://docs.docker.com/engine/security/apparmor/

Agree it shouldn't be too hard to hook this into nspawn... I don't really
use AppArmor or know it well though, so I'm not best placed to test it...

Cheers,
Filipe


On Thu, Apr 12, 2018 at 2:48 AM, Lennart Poettering 
wrote:

> On Di, 10.04.18 18:16, Matthias Pfau (matth...@tutanota.de) wrote:
>
> > Hi there,
> > we use apparmor on our production systems and want to test the setup in
> our test environment based on systemd-nspawn.
> >
> > Therefore, I installed apparmor on the host (debian stretch) and
> updated GRUB_CMDLINE_LINUX in /etc/default/grub to enable apparmor. I can
> use apparmor on the host system. However, within my containers, apparmor
> can not be started.
> >
> > `journalctl -kf` does not print anything when invoking `systemctl start
> apparmor` on the container and `systemctl status apparmor` just returns
> "ConditionSecurity=apparmor was not met".
> >
> > Is it possible to run apparmor in a container?
>
> Uh, I have no experience with AA but to my knowledge none of the
> kernel MACs (AA, SMACK, SELinux) are virtualized for container
> environments, i.e. there can only be one system policy, and containers
> tend to be managed under a single context only as a whole.
>
> But I'd be happy to be proved wrong, as I never touched AA, so I don't
> really know.
>
> If AA should indeed be virtualizable for containers then making nspawn
> support it is likely very easy, but I have my doubts it is...
>
> Please contact the AA community, and ask them whether AA containers
> can load their own policies. If yes, then please file an RFE issue
> against systemd, asking us to add support for this, with links to the
> APIs. best chance to get this implemented quickly would be to file a
> patch too, we'd be happy to review that.
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel
>


smime.p7s
Description: S/MIME Cryptographic Signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] how to login into a container booting with a minimal 'debian distro unstable' via nspawn

[Filipe Brandenburger]

Hi Florian,

On Sun, Mar 25, 2018 at 9:36 AM, Florian Held  wrote:
> how is it possible to log in into a container booting a minimal unstable
> debian distro via nspawn. After running:
>
> # debootstrap --arch=amd64 unstable ~/debian-tree/
> # systemd-nspawn -bD ~/debian-tree/
>
> prompts username followed by password. The combination
>
> "root"
> ""
>
> without quotes doesn't work. How can I login?

You can enter the container and just run a root shell on it with this command:

  # systemd-nspawn -D ~/debian-tree/ /bin/sh

(That's equivalent of single-user mode or a rescue shell on a machine.)

At that step, you can change the root password:

  # passwd root
  

At that point, boot the container again (with "-b") and you should be
able to log in.

I hope that helps!

Cheers,
Filipe


smime.p7s
Description: S/MIME Cryptographic Signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Filtering logs of a single execution of a (transient) service

[Filipe Brandenburger]

Hi!

So I'm testing a program repeatedly and using `systemd-run` to start a
service with it, passing it a specific unit name.

When the test finishes and I bring down the service, I want to be able to
collect the journald logs for that execution of the test alone.

Right now what I'm doing is naming the service differently every time,
including a random number, so I can collect the logs for that service alone
at the end. Such as:

  # myservice_name=myservice-${RANDOM}.service
  # systemd-run --unit="${myservice_name}" --remain-after-exit mybin
--myarg1 --myarg2 ...

And then collecting the logs using:

  # journalctl -u "${myservice_name}"

One disadvantage of this approach is that the units pile up as I keep
running tests...

  # systemctl status myservice-*.service

And that it's hard to find which one is the latest one, from an unrelated
session (this works only while active):

  # systemctl list-units --state running myservice-*.service

I would like to run these tests all under a single unit name,
myservice.service. I'm fine with not having more than one of them at the
same time (in fact, that's a feature.)

But I wonder how I can get the logs for a single execution...

The best I could come up with was using a cursor to get the logs for the
last execution:

  # journalctl -u myservice MESSAGE_ID=39f53479d3a045ac8e11786248231fbf
--show-cursor
  -- Logs begin at Thu 2018-03-22 22:57:32 UTC, end at Fri 2018-03-23
19:17:01 UTC. --
  Mar 23 16:40:00 mymachine systemd[1]: Started mybin --myarg1 --myarg2
  Mar 23 16:45:00 mymachine systemd[1]: Started mybin --myarg1 --myarg2b
  Mar 23 16:50:00 mymachine systemd[1]: Started mybin --myarg1 --myarg2
--myarg3
  -- cursor:
s=abcde12345...;i=123f45;b=12345abcd...;m=f123f123;t=123456...;x=...

And then use the cursor to query journald and get the logs from the last
execution:

  # journalctl -u myservice --after-cursor 's=abcde12345...;i=123f45;...'

That works to query the last execution of the service, but not a random
one...

I guess what I'm looking for is a way to get systemd to inject a journal
field to every message logged by my unit. Something like an environment
variable perhaps? Or some other field I can pass to systemd-run using -p.
Or something that systemd itself generates, that's unique for each
execution of the service and that I can query somehow (perhaps `systemd
show` while the service is up.) Is there any such thing?

Any other suggestions of how I should accomplish something like this?

Thanks!
Filipe


smime.p7s
Description: S/MIME Cryptographic Signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Cleanest way to halt a VM after a service has stopped

[Filipe Brandenburger]

Hi,

I found it's possible to halt a VM after a service has stopped by
using something like this:

ExecStopPost=/sbin/halt -p

Is this the cleanest approach? Or would anyone have a better
recommendation (perhaps using systemd-halt.service or similar)?

Thanks!
Filipe


smime.p7s
Description: S/MIME Cryptographic Signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] "StandardOutput=console" don't work as expected

[Filipe Brandenburger]

Hi,

On Wed, Dec 30, 2015 at 2:52 AM, Michael Chapman  wrote:
> On Wed, 30 Dec 2015, Reindl Harald wrote:
>>> >  i am asking for StandardOutput=console get piped to the terminal
>>> >  systemctl was called - the rest is done by crond as all the years
>>> > before
>>>
>>>  That isn't possible at the moment, and I doubt it will ever be
>>>  supported. The service is executed by systemd, not systemctl, and there
>>>  is no communication channel to return a stream of output from the
>>>  command back to systemctl.
>>
>> since "systemctl start" on the shell waits until the "oneshot" service is
>> finished it can't be impossible that pid 1 geives back the tasks output
>
> Well, it's "impossible" in the sense that systemd doesn't have any code to
> do that, no matter how much you might wish it did.

There's an item in the TODO list for `systemctl start -v ...` that
shows a log of the service in systemctl output:
https://github.com/systemd/systemd/blob/c57d67f718077aadee4e2d0940fb87f513b98671/TODO#L132

This might get closer to what you're suggesting here (even without
StandardOutput=console), though it's still about logs and not really
raw output.

As the TODO item suggests, it's tricky to implement correctly, so not
sure how long until it will be available...

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd 219 fails to create and/or use loop devices (or any other device)

[Filipe Brandenburger]

Hi,

On Thu, Nov 19, 2015 at 7:42 AM, von Thadden, Joachim, SEVEN
PRINCIPLES  wrote:
> using systemd 219-25 on Fedora 22 on a freshly created container I can not 
> make any
> device. Usage of --capability=CAP_MKNOD makes no difference.
>
> Steps to reproduce:
> [root@nbl ~]# machinectl pull-raw --verify=no
> http://ftp.halifax.rwth-aachen.de/fedora/linux/releases/21/Cloud/Images/x86_64/Fedora-Cloud-Base-20141203-21.x86_64.raw.xz
> [root@nbl ~]# systemd-nspawn --capability=CAP_MKNOD -M 
> Fedora-Cloud-Base-20141203-21.x86_64
> [root@Fedora-Cloud-Base-20141203-21 ~]# strace -f mknod /dev/loop0 b 7 0
> mknod("/dev/loop0", S_IFBLK|0666, makedev(7, 0)) = -1 EPERM (Operation not 
> permitted)

This is likely being caused by the use of the "devices" namespace,
which prevents you from using specific character and block devices
inside a cgroup. nspawn now sets these by default.

Calling systemd-nspawn with --property='DeviceAllow=/dev/loop0 rwm'
should allow it to mknod and later use it in losetup as well.

HTH!
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] network interface renaming via PCI ID w/ systemd-udevd

[Filipe Brandenburger]

On Thu, Nov 12, 2015 at 10:13 AM, Matthew Hall  wrote:
> On Thu, Nov 12, 2015 at 10:37:56AM +0100, Lennart Poettering wrote:
>> Since time began eth* is where the kernel automatically picked iface
>> names from. If you want to assign your own names go for some other
>> namespace, or be prepared to race against the kernel, and deal with
>> it.
>
> Again, this logic worked well when the level of dynamism was lower.

I think I see where you're coming from... Some distributions (in my
recollection, RHEL) would use weird tricks to keep interface ordering
stable while still keeping the eth0, eth1, ethX names.

If I recall correctly, RHEL's /etc/init.d/network would try to match
interface names from /etc/sysconfig/networking-scripts/ifcfg-ethX to
the MAC address listed inside that configuration file. If it had to
switch it from, say, eth0 to eth1, it would do weird tricks such as
looking whether eth1 existed already, then rename it to tmp98765 with
a random number, then rename eth0 to eth1. In many cases, something
would go awry and you would end up with an interface named tmp98765.

As you can imagine, this was fraught with problems and race
conditions. It doesn't really work when you're trying to boot with as
much parallelism (which is something we aim for these days) or even
hot plug new interfaces...

> But now the level of dynamism is higher and different principles should apply.

Yes. I'd say that's a good thing.

> You aren't thinking very much about how it will work for newer users.

New users mostly don't care... I really think retraining your fingers
from eth -> enp or whatever you pick, net, lan, wired, etc. is
probably much easier than trying to preserve a relic of a name that
mostly serves no purpose these days... As mentioned, keeping it is not
simple since it's still the dumping ground for the kernel (and that's
unlikely to change), avoiding the race with the kernel is much better
than trying to deal with it, the complexity is just not worth it... I
hope you get to see the light!

Cheers!
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd and intltool

[Filipe Brandenburger]

Hi,

On Thu, Sep 10, 2015 at 10:10 AM, Michael Biebl  wrote:
> reading https://wiki.gnome.org/Projects/GnomeCommon/Migration, it says
> that intltool is practically dead and one should use gettext directly.
>
> Do we still need intltool in systemd? Does gettext have support for
> translating PolicyKit policy files?

I'm a fan of the suggestion to remove intltool!

Not sure if the page you link too really indicates intltool itself is
obsolete, I think what they meant is that it's obsolete for GNOME
projects...

When I looked at it last, I opened a bug in systemd tracker to remove
intltool dependency:
https://bugs.freedesktop.org/show_bug.cgi?id=79692

And they suggested opening a bug to request native PolicyKit support in gettext:
http://savannah.gnu.org/bugs/index.php?42615

But I'm not really sure that went anywhere...

At the hackfest of LPC '14 I heard of the plans to replace PolicyKit
with another component (not really based on XML), so I was hoping this
would end up solving this problem, not sure whether those plans still
exist, what is the progress and whether we could help there? Or should
we look again at alternative approaches to removing the intltool
dependency?

Cheers!
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [ANNOUNCE] systemd v221

[Filipe Brandenburger]

Guys let's try to be constructive here...

This time it shouldn't be too painful for downstreams since the revert
was the last patch to the man subtree so just a git revert of that
should get your trees to the state you need to get v221 packages for
Debian and Ubuntu. In that sense, I think we're still (slightly)
better off than we were in v220 and I think we have all we need to
solve this one for good in v222.

And let's use the momentum to try to solve this soon, in which case
you could even replace the revert of that commit with the backport of
the next one (which will probably remove the disted manpages).

Cheers,
Filipe


On Fri, Jun 19, 2015 at 7:40 AM, Michael Biebl mbi...@gmail.com wrote:
 2015-06-19 16:38 GMT+02:00 Lennart Poettering lenn...@poettering.net:
 On Fri, 19.06.15 16:29, Michael Biebl (mbi...@gmail.com) wrote:

  If something is not in shape we'll revert it. Regardless of the
  general merits of the patch set: this one actually broke stuff, it
  was incomplete. Either you make the man pages dynamic, or you ship
  them pre-built. The patch set did both. That's broken, and hence has
  no place in a release. And I'd much rather see that stuff removed
  again than having to delay the release further.
 
  Not amused, not amused at all.
 
  I am sorry you feel that way.
 
  I am not sure though what you suggest: delay releases until zero
  bugfixes have been applied for a week? Well, that would mean we'd
  never do releases again, sorry. We have to release some time. On

 Bullshit. That's been in master for a while and the justification for
 reverting appear to totally made up.

 I wasn't the one who reverted it or even involved in the
 discussions. But what I saw is that the patch was borked, since it one
 one hand tried to ship the man pages pre-built but also wanted to make
 them different depending on ./configure runs. And that's just
 *broken*.

 The justification for the revert basically boil down to:
 let's make it as hard as possible and use systemd as a stick to force
 them to not use split-usr.

 The arguments for the for the patch being broken are completely made up.


 --
 Why is it that all of the instruments seeking intelligent life in the
 universe are pointed away from Earth?
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Performance of systemctl status tab completion

[Filipe Brandenburger]

On Thu, Jun 18, 2015 at 11:54 AM, Lennart Poettering
lenn...@poettering.net wrote:
 On Tue, 02.06.15 12:18, Chris Morgan (chmor...@gmail.com) wrote:
 systemd 216 here on an embedded arm system, 1ghz with a load of 60% or
 more. I enabled tab completion, because I really don't like to type,
 and quickly found out that something like:

 systemctl status xxtab tab

 Takes a really long time to complete. In some cases something like 20+ 
 seconds.

 Hmm, I figure we rely on some more profiling info from you to do
 something about this, i.e. is this io bound or cpu bound? It would be
 good to get some data from the perf tool for this.

BTW, you probably don't need to go all the way down to using perf to
get profiling data, just use a set -x on your shell which will
expand the commands it's running for completion so you might be able
to spot where the bottleneck actually is...

HTH,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [ANNOUNCE] Git development moved to github

[Filipe Brandenburger]

On Wed, Jun 10, 2015 at 9:52 AM, Lennart Poettering
lenn...@poettering.net wrote:
 On Wed, 10.06.15 08:25, Filipe Brandenburger (filbran...@google.com) wrote:
 On Wed, Jun 10, 2015 at 6:31 AM, Alban Crequy al...@endocode.com wrote:
  FWIW it only loses the comments if people comment on individual
  commits instead of commenting on the Files changed tab of a PR. I
  usually comment in this way on purpose instead of commenting on
  commits, so that the history of comments are kept in the PR, even
  after rebase (it might be folded if the chunk of the patch is not
  there anymore, but the comment is still in the PR). If you really want
  to comment on an individual commit (but I don't recommend it), you can
  include the reference of the PR in your comment (#42), then github
  will keep your comment attached to the PR.

 Ah that makes sense!

 Indeed as I explained I like to look at the individual commits, so
 that would explain why my comments would get lost as a new version is
 pushed...

  I think it is fine as it is as long as people comment in the Files
  changed tab.

 Lennart, do you think setting that rule is better than the one PR per
 version of patchset?

 No. We should review commits, not diffs. We also should review commit
 msgs. (see other mail)

Another downside of adding comments to the commits is that e-mail
notifications are not sent for them (I just noticed that while lurking
on #164, I got e-mails for the main thread but not for Lennart's
comments on commit 5f33680.)

I think adding comments on the Files changed would work on cases such as:

1) The PR contains only a single commit, in which case the diff in
Files changed will match the commit itself. (You still need to look
at the commit description, but even if you do it from the Commits
tab you can't really add any line comments directly to it anyways.)

2) If the commits change disjoint sets of files (you could check that
first, and then review the code in the Files changed tab.)

I think the exception is when a PR is both introducing new code and
later changing it in a follow up commit but I guess that's not really
too frequent (though I'm clearly guilty of it on #44.)

Can we try to add comments to Files changed? Not asking not to look
at the commits, yes looking at the commits is important! It's just
that I think if we could have the e-mail notifications for the line
comments, make sure they are kept in the same thread and be able to
keep multiple versions of a patchset around in the same PR (instead of
the wonky PR linking) I think that would be a huge win... We can
always fall back to opening a new PR and closing the old one, but I'd
prefer if that was the exception and not the rule...

It really sounds like what you really want is Gerrit... I think
gerrithub.io (which I haven't tried personally) might be what bridges
these two worlds... Makes it easy for the submitters to send you
commits, makes it easy for the reviewers to adopt the new code, tracks
pending requests.

Cheers!
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [ANNOUNCE] Git development moved to github

[Filipe Brandenburger]

On Thu, Jun 11, 2015 at 12:31 PM, Ronny Chevalier
chevalier.ro...@gmail.com wrote:
 On Thu, Jun 11, 2015 at 6:31 PM, Filipe Brandenburger filbran...@google.com 
 wrote:
 Another downside of adding comments to the commits is that e-mail
 notifications are not sent for them (I just noticed that while lurking
 on #164, I got e-mails for the main thread but not for Lennart's
 comments on commit 5f33680.)

 Yes you need to specify for each PR you are interested in that you
 want to receive mail notifications for the PR... (I think it's the
 subscribe button at the bottom)

Sorry I should have explained myself better...

I watch systemd/systemd as a whole, so I get all notifications
without having to ask for them individually...

On #164, I *did* get an e-mail for @zonque's comment (Also, you
forgot to add the new files to Makefile.am and po/LINGUAS...) but I
did *not* get e-mails for @poettering's comments on commit 5f33680
(Hmm, can you please change the commit msg to say this is the catalog
translation? ...) and the replies on that thread (@s8321414 replied
@poettering How can I do this using git? etc.)

I think that's one more symptom of the fact that, for GitHub, the
commit itself doesn't directly belong to the PR, and so does not
belong to the project either...

The e-mail notifications are not really a great big deal (still, they
are annoying), but I think it's just one more sign that adding
comments to the commits will end up causing trouble in the future...

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [ANNOUNCE] Git development moved to github

[Filipe Brandenburger]

On Wed, Jun 10, 2015 at 5:04 AM, Martin Jansa martin.ja...@gmail.com wrote:
 If you want good review tool, why not use gerrit?

+1 for Gerrit as a code review tool.

It's not perfect, but from all of them that I've used it seems to get
the most right:
- Review *commits* and not PRs (tends to drives up the quality of
individual commits and commit descriptions.)
- Linear history by default (though you can still configure it to
create the merges if you really care about preserving the original
commit ids.)
- Easy to compare two versions of the same commit, you can even do
side-by-side diff of version 3 with version 7 or whatever you like.
- You get to do line-by-line comments on the commit description, so
easy to nitpick there as well.
- You can take over commits from someone else and push a new version
*in the same review* so the comments are preserved and you can still
do side-by-side diffs.
- It does not tend to create spurious branches in the main project,
such as revert-*, etc. When doing `git fetch` we get a copy of those
which just serves to create noise.

Yes I noticed there's gerrithub.io, I haven't really looked at them
closely but looks like a good alternative...

They say they can convert PRs into Gerrits, I just don't know how easy
it is to update the commits with the comments, whether they can still
figure that out from the new push to the same branch of the PR or if
in that case you *need* to use Gerrit for it to figure out the
Commit-Id's, etc...

I'd be willing to do a more thorough evaluation of gerrithub.io, looks
like it can be used in parallel with GitHub anyways.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Messed up PR references on Github

[Filipe Brandenburger]

On Wed, Jun 10, 2015 at 12:23 AM, Jan Synáček jsyna...@redhat.com wrote:
 See https://github.com/systemd/systemd/pull/5. There are multiple
 references to this PR that say user referenced this pull request from
 a commit in commit, which is hilarious, as those clearly are not
 references to this PR. Their commit messages contain the string #5 and
 Github thinks it means a reference. I'm pretty sure this will mess up a
 lot of pull requests in the future. Is there a way to fix this?

If you learn one thing about Markdown (or GitHub Markdown), learn that
a block delimited by ``` lines makes it a verbatim literal (think
pre in HTML terms), so when pasting output the best is:

```
#1 bla bla bla
#2 yada yada
```

That won't expand the #n references or any other GitHub syntax...

There's always the Preview tab which is useful to look before
submitting the comment as well...

And I think you can always edit your comments after you posted them
(not sure if that will undo the link between the two PRs though.)

HTH,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [ANNOUNCE] Git development moved to github

[Filipe Brandenburger]

On Wed, Jun 10, 2015 at 6:31 AM, Alban Crequy al...@endocode.com wrote:
 Instead, just reuse the same PR and use `git push -f` to ship new
 versions of the commits to the same branch... Yes it's awful but
 unfortunately that's how GitHub works...

 Yeah, it is awful, and loses all the comments, as well is incompatible
 with having multiple people making patch suggestions for the same
 issue.

 FWIW it only loses the comments if people comment on individual
 commits instead of commenting on the Files changed tab of a PR. I
 usually comment in this way on purpose instead of commenting on
 commits, so that the history of comments are kept in the PR, even
 after rebase (it might be folded if the chunk of the patch is not
 there anymore, but the comment is still in the PR). If you really want
 to comment on an individual commit (but I don't recommend it), you can
 include the reference of the PR in your comment (#42), then github
 will keep your comment attached to the PR.

Ah that makes sense!

Indeed as I explained I like to look at the individual commits, so
that would explain why my comments would get lost as a new version is
pushed...

 I think it is fine as it is as long as people comment in the Files
 changed tab.

Lennart, do you think setting that rule is better than the one PR per
version of patchset?

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [ANNOUNCE] Git development moved to github

[Filipe Brandenburger]

Moving from #88 to this thread:

On Tue, Jun 9, 2015 at 12:41 PM, Lennart Poettering
lenn...@poettering.net wrote:
 So I think updating the PR (by force-pushing) is really nasty, and we
 shouldn't do it. Instead, please push a new PR, mention that it
 obsoletes the old one. (of course, I wished that github would know a
 concept of PR obsoletion natively...)

 Also, I think in this case we really should require an issue being
 created for the entire thing that groups the various PRs together. (In
 fact, I am pretty sure we should *enforce* that all PRs have an issue
 attached to them. Maybe with a bot or so that creates issues for all
 PRs that reference no issue).

I don't think it's very practical to require opening multiple pull
requests for the several revisions and requiring that authors do not
use git push -f, particularly since most users of GitHub are already
used to that... Also, requiring an open issue to keep track of the
multiple PRs will probably quickly produce a sea of ids that will be
really hard to keep track of.

I think a more productive advice would be for reviewers to avoid using
line comments for anything that is wanted for posterity and instead
only use them to say typo or comment here or to point out what
exactly in the code the comment on the main thread is making reference
to. Wouldn't you think that would be preferrable?

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [ANNOUNCE] Git development moved to github

[Filipe Brandenburger]

On Tue, Jun 9, 2015 at 12:59 PM, Lennart Poettering
lenn...@poettering.net wrote:
 [...] so we comment and ask for a new PR, and close the old one.

See my previous comment, I think this cure is worse than the disease :-)

Instead, just reuse the same PR and use `git push -f` to ship new
versions of the commits to the same branch... Yes it's awful but
unfortunately that's how GitHub works...

To work around the problem of line comments being lost, just ask
*reviewers* to make most of the relevant comments in the PR thread and
keep line comments to simple comments that are probably not going to
be relevant when they're obliterated...

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [ANNOUNCE] Git development moved to github

[Filipe Brandenburger]

On Tue, Jun 9, 2015 at 2:37 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Tue, 09.06.15 13:04, Filipe Brandenburger (filbran...@google.com) wrote:
 On Tue, Jun 9, 2015 at 12:59 PM, Lennart Poettering lenn...@poettering.net 
 wrote:
  [...] so we comment and ask for a new PR, and close the old one.

 See my previous comment, I think this cure is worse than the
 disease :-)

 Why would you say this? Why are multiple sequencial PR, where the old
 obsoleted ones are closed and locked that bad?

- Too much administrivia
- Threads get split up (did I comment on the origina PR or on this one?)
- Hard to follow the references around
- E-mail notifications get split into separate threads (not that
GitHub does a stellar job of e-mail notifications anyways...)
- ...

I actually think the fact that in GitHub you'll use a PR *or* and
Issue is actually good, so you mainly have a single thread to discuss
the same item...

I just think that working around the GitHub bug of losing comments by
creating a convoluted workflow around it (which is hard to enforce, as
you can't really block PR authors from using `git push -f`) is the
wrong approach...

Maybe someone should complain to GitHub about this issue with losing
track of the comments on the previous versions of the code instead?

If that was fixed in GitHub, would you be happy not splitting multiple
PRs for multiple versions of the same feature/issue?

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [ANNOUNCE] Git development moved to github

[Filipe Brandenburger]

Hi,

On Tue, Jun 2, 2015 at 7:58 AM, Dimitri John Ledkov
dimitri.j.led...@intel.com wrote:
 And I think this is _good_, because the submitter's commit ids will be
 preserved (together with the signed gpg commits) [...]

This, signed gpg commits, is actually the first reasonable argument I
see for merging and not rebasing commits.

TL;DR: GitHub model sucks, but I think we can live with it.

I think, though, in general, the GitHub way of focusing on PRs and
not commits tends to generate poorer git commits and git histories in
general. I too often see broken PRs being ammended with second or
third commits to fix the bugs, which makes git bisect hit and miss in
lots of projects. And many projects have commit descriptions that are
totally meaningless (that's not just a GitHub thing, but I think
GitHub makes it a lot easier to get sloppy on those.)

I actually think who gets it right is Gerrit which is focused on
individual commits and not in PRs with sets of commits. Even if you
upload 3 or 4 commits in a row, they'll be reviewed one by one and
submitted as they're approved. It's easy to merge the first two but
send the last two back for rework. With GitHub, you'd have to do that
manually. I'm not saying Gerrit is perfect, far from it, but I think
at least they got it right in being commit-centric rather than
PR-centric.

GitHub also makes it hard to look at the individual commits and commit
messages. By default, they just give you the blurb the author typed on
the PR description (which ends up nowhere in git) and show you a
consolidated diff for the whole PR, so it's quite possible that the
first commit in the series doesn't even compile and you won't really
get to know about that as you merge it. I always make a point to first
thing reviewing a PR go to the commits tab and look at each commit
on its own, most of the time I don't even look at the consolidated
diff since I think it's mostly meaningless.

I'm of course counting on our maintainers here to make a good job of
weeding out bad commits.

I think one model that helps is that of the Commit Queue, in which
whenever a commit (or PR) is ready to be merged, it still goes through
a queue that tries to build it and possibly run some smoke tests to
ensure nothing is badly broken before actually merging it to git. Such
a system would most likely rebase, producing a linear history.

Oh, and I'm all for merges of feature branches, for an actual
*feature* as in something that takes 15 commits to implement and
needs to be developed in a separate branch. But those will most likely
be merged manually since I don't expect they won't have conflicts
anyways, so the PR merge feature of GitHub will be mostly useless
then...

Anyways, excuse the rant... I think we can live with GitHub and in the
end, everything will be alright :-)

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Bogus issue/50 branch on systemd git at GitHub

[Filipe Brandenburger]

I don't think it should be there...

https://github.com/systemd/systemd/branches

Otherwise everyone doing git fetch will get a copy of this branch...

I think the main git should only expose a single master branch.

Would you care to delete it and be careful not to push extraneous
branches to the main repo?

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [RFC] load-fragment: use unquote_first_word in config_parse_exec

[Filipe Brandenburger]

Hi,

This commit was not moved to GitHub, it's under review here:
https://github.com/systemd/systemd/pull/44

On Sun, May 31, 2015 at 12:06 AM, Andrei Borzenkov arvidj...@gmail.com wrote:
 В Sat, 30 May 2015 23:29:29 -0700 Filipe Brandenburger 
 filbran...@google.com пишет:
 - Handling a \; is ugly, it looks like a hack... unquote_first_word is
   not equipped to recognize that sequence, so I had to move it outside
   unquote_first_word looking for those two characters followed by
   whitespace explicitly. But then, something like ';' or ; will be
   recognized as a command separator, is that OK?

 I do not think it's OK. Having quoted string act like a separator is
 definitely unexpected and confusing.

Addressed. Now ; or ';' will turn into a literal semicolon, so will
\; only when it's on its own, and only a single ; on its own will be
recognized as a command separator.

 - We do something different for empty string (clear the command list)
   than we do for just whitespace. This is pre-existing. Maybe we need to
   fix that? I put a comment on that case, that branch is triggered both
   in the just whitespace case as well as right after a semicolon
   command separator.

 Not sure I understand what you mean. Do you suggest that

 ExecStart=/usr/bin/foo ; ; /usr/bin/bar

 should discard /usr/bin/foo? Or that it does it already?

I added a test case to cover two semicolons in a row. With the new
code it breaks as it thinks ; is a new command name, I think that's
appropriate enough, let me know if you think otherwise.

Cheers!
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [systemd-commits] load-fragment: use UNESCAPE_RELAX flag to parse exec directives

[Filipe Brandenburger]

Pull Request created:
https://github.com/systemd/systemd/pull/44

If you could test it and confirm your sudo case with \073 works now,
and your regexp case isn't broken, I'd appreciate it.

Cheers,
Filipe


On Tue, Jun 2, 2015 at 2:53 PM, Filipe Brandenburger
filbran...@google.com wrote:
 Hi Michael,

 On Tue, Jun 2, 2015 at 2:50 PM, Michael Biebl mbi...@gmail.com wrote:
 2015-06-02 12:55 GMT+02:00 Daniel Mack dan...@zonque.org:
 So, my primary motivation was to fix the obvious regression at hand
 first, but I agree the actual problem goes deeper.

 Looks like even with this patch applied, we have regressions compared
 to previous releases.
 See https://bugs.freedesktop.org/show_bug.cgi?id=90794#c4

 So that looks like a \073 being turned into, well, \\073, instead of
 turning into ;.

 This is actually fixed in my latest patch that uses unquote_first_word.

 I still want to run some additional tests on it and add some extra use
 cases, but that use case for \073 is already there and was actually
 the one I had to fix to do ; instead.

 I expect to be able to send this patch out later today or early tomorrow.

 Cheers,
 Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [systemd-commits] load-fragment: use UNESCAPE_RELAX flag to parse exec directives

[Filipe Brandenburger]

Hi Michael,

On Tue, Jun 2, 2015 at 2:50 PM, Michael Biebl mbi...@gmail.com wrote:
 2015-06-02 12:55 GMT+02:00 Daniel Mack dan...@zonque.org:
 So, my primary motivation was to fix the obvious regression at hand
 first, but I agree the actual problem goes deeper.

 Looks like even with this patch applied, we have regressions compared
 to previous releases.
 See https://bugs.freedesktop.org/show_bug.cgi?id=90794#c4

So that looks like a \073 being turned into, well, \\073, instead of
turning into ;.

This is actually fixed in my latest patch that uses unquote_first_word.

I still want to run some additional tests on it and add some extra use
cases, but that use case for \073 is already there and was actually
the one I had to fix to do ; instead.

I expect to be able to send this patch out later today or early tomorrow.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [systemd-commits] load-fragment: use UNESCAPE_RELAX flag to parse exec directives

[Filipe Brandenburger]

On Tue, Jun 2, 2015 at 3:55 AM, Daniel Mack dan...@zonque.org wrote:
 The problem here is that cunescape() wasn't as strict in the past as it
 should have been, and now there are unit files in the wild which contain
 escape sequences that slip through the C unescaping mechanism.

 So, my primary motivation was to fix the obvious regression at hand
 first, but I agree the actual problem goes deeper.

Yeah I didn't realize it was actually a regression... And looking back
at that case from the bug, the quotes systemd sees is actually double
quotes.

Not to mention that I talked about differences of single and double
quotes in shell but that doesn't really make sense because the shell
doesn't really expand \r or \n etc. (I think I was probably confusing
it with how quotes work in... well, Perl maybe? It doesn't really
matter.)

 I think that makes sense, but we need to make sure not to break existing
 setups. So for instance, both

   ExecStart=-/bin/sh -c echo foo@0 | grep -Po '\w+@\K[\d.]+'
   ExecStart=-/bin/sh -c echo foo@0 | grep -Po '\\w+@\\K[\\d.]+'

 are currently allowed and lead to the same result. The former because
 with UNESCAPE_RELAX, all sequences in the string (\w, \K, \d) are
 unrecognized and copied literal, and the latter because double
 backslashes become single ones.

Ok, I think that makes sense, we need to support those two use cases working.

So I think I'll go back to unquote_first_word and add support for
UNESCAPE_RELAX to it. I'm thinking if unquote_first_word receives
UNQUOTE_RELAX, then it should do the equivalent of UNESCAPE_RELAX to
its calls to cunescape_one. I think that would be appropriate here.

I think, as a bonus, I'll get some more use cases to work unchanged
and go back from now returning -EINVAL to returning 0 again, so
probably fewer regressions here.

 Martin already provided a test case the cases in the bug reports, and we
 should probably add more to make sure we don't cause more regressions
 with your rework.

 Do you have any patches in the queue for changes you proposed?

Yes, but I'll have to go back and add support to unquote_first_word to
do the equivalent of UNESCAPE_RELAX, which is where I stopped on it
yesterday... I think it's workable though.

I should have a reviewed patchset later today or tomorrow, I'll send
it as a GitHub PR.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 0/2] Using XML entities for paths in manpages

[Filipe Brandenburger]

Hi Daniel,

On Tue, Jun 2, 2015 at 3:04 AM, Daniel Mack dan...@zonque.org wrote:
 On 06/02/2015 11:25 AM, Martin Pitt wrote:
 FTR, this works fine here, using --with-rootprefix= (to avoid the
 extra slashes). This spawned a long thread and multiple followup
 patches, and TBH I lost track which patches got proposed and which are
 superseded; but this one looks good to me. Thank you!

 Michael came up with a nicer approach to sanitize all path arguments to
 configure and strip trailing slashes before the Makefile variables are
 substituted. I think we should go for that. I'll collect patches and
 prepare a pull request later.

This is the one patch which does path AX_NORMALIZE_PATH that we're
still missing:
https://github.com/systemd/systemd/commit/d10b3a45b17403ce8a52680703b03888ebee0769

I resent Michael's original patch but including the import of the
latest ax_normalize_path.m4 from autoconf-archive.

I also tested it with a few combinations including --with-rootpath=/
and it worked fine.

I could create a PR in systemd-devs GitHub if you'd like, otherwise
feel free to just push it straight upstream if you prefer.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 0/2] Using XML entities for paths in manpages

[Filipe Brandenburger]

On Tue, Jun 2, 2015 at 7:24 AM, Daniel Mack dan...@zonque.org wrote:
 I could create a PR in systemd-devs GitHub if you'd like, otherwise
 feel free to just push it straight upstream if you prefer.

 Nope, let's try to get used to the new workflow. Just create the PR :)

Good call... we were actually missing two patches, the second one was
yours replacing the /usr/lib cases with rootprefix...

GitHub PR is here:
https://github.com/systemd/systemd/pull/39

Michael Biebl, I wanted to cc you on the GitHub PR but didn't find you
there, feel free to cc yourself by following that URL. In particular,
take a look at your commit which I adapted to include the .m4 file
from autoconf-archive.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [systemd-commits] load-fragment: use UNESCAPE_RELAX flag to parse exec directives

[Filipe Brandenburger]

Hi,

Not sure I agree with the commit below. (In particular as I'm looking
at converting this code into using unquote_first_word.)

On Mon, Jun 1, 2015 at 9:10 AM, Daniel Mack
zon...@kemper.freedesktop.org wrote:
 commit 22874a348fb1540c1a2b7907748fc57c9756a7ed
 Author: Daniel Mack dan...@zonque.org
 Date:   Mon Jun 1 17:49:04 2015 +0200

 load-fragment: use UNESCAPE_RELAX flag to parse exec directives

 The cunescape() helper function used to handle unknown escaping sequences
 gracefully by copying them over verbatim.

 Commit 527b7a42 (util: rework cunescape(), improve error handling) added
 a flag to make that behavior optional, and changed to default to error out
 with -EINVAL otherwise.

 However, config_parse_exec(), which is used to parse the
 Exec{Start,Stop}{Post,Pre,} directives of unit files, was not changed 
 along
 with that commit, which means that directives with improperly escaped
 command line strings are no longer parsed.

 Relevant bugreports include:

   https://bugs.freedesktop.org/show_bug.cgi?id=90794
   https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=787256

 Fix this by passing UNESCAPE_RELAX to config_parse_exec() in order to
 restore the original behavior.

 diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
 index c95c110..df5fe6f 100644
 --- a/src/core/load-fragment.c
 +++ b/src/core/load-fragment.c
 @@ -610,7 +610,7 @@ int config_parse_exec(
  else
  skip = strneq(word, \\;, MAX(l, 1U));

 -r = cunescape_length(word + skip, l - skip, 0, c);
 +r = cunescape_length(word + skip, l - skip, 
 UNESCAPE_RELAX, c);
  if (r  0) {
  log_syntax(unit, LOG_ERR, filename, line, r, 
 Failed to unescape command line, ignoring: %s, rvalue);
  r = 0;

So, my problem with it is that the bug's expectation is that
backslashes inside single quotes will remain as backslashes, as the
example is a regexp '\w+@\K[\d.]+'.

But this is not true here!!! It's only fixing it for the particular
cases that are not escape sequences yet.

For instance, what if I'm doing a parameter that is a regexp that is
looking for a word boundary and I want to use '\b'? systemd with the
current patch will (still) turn this into a backspace character.

Right now the systemd quoting rules do *not* match the shell quoting
rules. (In fact, this is akin to a bug complaining that variables in
systemd do not match shell variables. That's indeed the case, but it
doesn't make it a bug. It's working as documented and as intended.)

I'd be ok with changing the rules so that backslash inside single
quotes remains a literal backslash, as I think we have the two kinds
of quotes (single quotes and double quotes) and I don't think it would
hurt to make them work a little bit closer to how the shell works...
(Though we'll keep expanding variables inside single quotes?)

In that case (of making backslashes stay literal inside single quotes)
I think the best way forward is complete the conversion to
unquote_first_word and then update unquote_first_word to introduce
those rules (essentially, just get rid of the SINGLE_QUOTE_ESCAPE rule
would do.)

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [RFC] load-fragment: use unquote_first_word in config_parse_exec

[Filipe Brandenburger]

Hi,

On Sun, May 31, 2015 at 12:06 AM, Andrei Borzenkov arvidj...@gmail.com wrote:
 В Sat, 30 May 2015 23:29:29 -0700 Filipe Brandenburger 
 filbran...@google.com пишет:
 - Handling a \; is ugly, it looks like a hack... unquote_first_word is
   not equipped to recognize that sequence, so I had to move it outside
   unquote_first_word looking for those two characters followed by
   whitespace explicitly. But then, something like ';' or ; will be
   recognized as a command separator, is that OK?

 I do not think it's OK. Having quoted string act like a separator is
 definitely unexpected and confusing.

I believe this matches the current behavior, having a single ; or
';' become a command separator, only \; is recognized as a valid
escape. The reason is that both FOREACH_WORD_QUOTED and
unquote_first_word will simply return the unquoted string, so there is
no good way to differentiate ; from ; or ';'.

We could work around that (after the conversion to unquote_first_word)
similarly to how \; is being handled, by looking at the next
characters of p to check if p[0] == ';' and p[1] is whitespace and
handle that in particular.

 - We do something different for empty string (clear the command list)
   than we do for just whitespace. This is pre-existing. Maybe we need to
   fix that? I put a comment on that case, that branch is triggered both
   in the just whitespace case as well as right after a semicolon
   command separator.

 Not sure I understand what you mean. Do you suggest that

 ExecStart=/usr/bin/foo ; ; /usr/bin/bar

 should discard /usr/bin/foo? Or that it does it already?

 The use case is to clear any pre-existing value in drop-in like

 ExecStart=
 ExecStart=new command line

So, my read of the current code makes me believe that
ExecStart=empty will indeed clear any pre-existing value but on the
other hand ExecStart=whitespace will not. That's what I'm referring
to.

I'm not really sure how the current code behaves to two semicolons in
a row, like your /usr/bin/foo ; ; /usr/bin/bar example. The code
after my patch will probably try to handle the second ; as a command
name and will most likely choke on it at the very least because it's
not an absolute path. I haven't really tested either in particular
though...

Sorry if I didn't make it clear... but in most cases the limitations I
mentioned already existed in the old code. I just want to make sure
that the new code is not more buggy or buggy in worse ways than the
old code. I believe addressing these shortcomings in the new code
should be more straightforward. In particular, I plan to start looking
at the empty vs. whitespace issue and maybe have a follow up commit to
this one that addresses that.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] util: Refactor common cunescape block in unquote_first_word

[Filipe Brandenburger]

Get rid of code duplication here.

---
Some alternatives I considered:

1) Keep a separate bool escape and set it when we see a backslash, still
   keeping state set to VALUE or SINGLE_QUOTE or DOUBLE_QUOTE.

2) Create the enum so that ESCAPE has a bit value like 0x100 or similar, so
   that we can use a bitwise operation such as state = ~ESCAPE to go back to
   the unescaped state.

Not sure if we want to differentiate between backslash escapes inside single,
double quotes or unquoted in the future... But we can always split this code
back again in the future, or handle specific exceptions inside this same block.

Let me know if you'd like me to rework this to use one of the two suggested
alternate approaches above.

Cheers!
Filipe

 src/shared/util.c | 68 ++-
 1 file changed, 7 insertions(+), 61 deletions(-)

diff --git a/src/shared/util.c b/src/shared/util.c
index 8a6107969ae1..5b03fde1143c 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -5200,35 +5200,6 @@ int unquote_first_word(const char **p, char **ret, 
UnquoteFlags flags) {
 
 break;
 
-case VALUE_ESCAPE:
-if (c == 0) {
-if (flags  UNQUOTE_RELAX)
-goto finish;
-return -EINVAL;
-}
-
-if (!GREEDY_REALLOC(s, allocated, sz+7))
-return -ENOMEM;
-
-if (flags  UNQUOTE_CUNESCAPE) {
-uint32_t u;
-
-r = cunescape_one(*p, (size_t) -1, c, u);
-if (r  0)
-return -EINVAL;
-
-(*p) += r - 1;
-
-if (c != 0)
-s[sz++] = c; /* normal explicit char */
-else
-sz += utf8_encode_unichar(s + sz, u); 
/* unicode chars we'll encode as utf8 */
-} else
-s[sz++] = c;
-
-state = VALUE;
-break;
-
 case SINGLE_QUOTE:
 if (c == 0) {
 if (flags  UNQUOTE_RELAX)
@@ -5247,35 +5218,6 @@ int unquote_first_word(const char **p, char **ret, 
UnquoteFlags flags) {
 
 break;
 
-case SINGLE_QUOTE_ESCAPE:
-if (c == 0) {
-if (flags  UNQUOTE_RELAX)
-goto finish;
-return -EINVAL;
-}
-
-if (!GREEDY_REALLOC(s, allocated, sz+7))
-return -ENOMEM;
-
-if (flags  UNQUOTE_CUNESCAPE) {
-uint32_t u;
-
-r = cunescape_one(*p, (size_t) -1, c, u);
-if (r  0)
-return -EINVAL;
-
-(*p) += r - 1;
-
-if (c != 0)
-s[sz++] = c;
-else
-sz += utf8_encode_unichar(s + sz, u);
-} else
-s[sz++] = c;
-
-state = SINGLE_QUOTE;
-break;
-
 case DOUBLE_QUOTE:
 if (c == 0)
 return -EINVAL;
@@ -5292,7 +5234,9 @@ int unquote_first_word(const char **p, char **ret, 
UnquoteFlags flags) {
 
 break;
 
+case SINGLE_QUOTE_ESCAPE:
 case DOUBLE_QUOTE_ESCAPE:
+case VALUE_ESCAPE:
 if (c == 0) {
 if (flags  UNQUOTE_RELAX)
 goto finish;
@@ -5312,13 +5256,15 @@ int unquote_first_word(const char **p, char **ret, 
UnquoteFlags flags) {
 (*p) += r - 1;
 
 if (c != 0)
-s[sz++] = c;
+s[sz++] = c; /* normal explicit char */
 else
-sz += utf8_encode_unichar(s + sz, u);
+sz += utf8_encode_unichar(s + sz, u); 
/* unicode chars we'll encode as utf8 */
 } else
 s[sz++] = c;
 
-state = DOUBLE_QUOTE;
+state = (state == SINGLE_QUOTE_ESCAPE) ? SINGLE_QUOTE :
+   

[systemd-devel] [RFC] load-fragment: use unquote_first_word in config_parse_exec

[Filipe Brandenburger]

This is an attempt to convert it from using FOREACH_WORD_QUOTED into a
loop using unquote_first_word repeatedly.

Additionally, we're looping through the arguments only once and using
_cleanup_ functions to manage objects owned by this function.

Some notes:
- There is some difference in how unquote_first_word handles invalid
  quotes or escapes. In particular, for some cases where we used to
  return 0 (but set the command to NULL) we are now returning -EINVAL
  instead (still setting the command to NULL.)  I don't know if that's a
  problem anywhere...

- The test cases were updated to reflect that change.

- Handling a \; is ugly, it looks like a hack... unquote_first_word is
  not equipped to recognize that sequence, so I had to move it outside
  unquote_first_word looking for those two characters followed by
  whitespace explicitly. But then, something like ';' or ; will be
  recognized as a command separator, is that OK?

- We do something different for empty string (clear the command list)
  than we do for just whitespace. This is pre-existing. Maybe we need to
  fix that? I put a comment on that case, that branch is triggered both
  in the just whitespace case as well as right after a semicolon
  command separator.

- Now we're logging more from this function, in particular if we run out
  of memory we return log_oom(), but that might not be the case for
  instance if we get -ENOMEM from unquote_first_word. Should we wrap
  around that if (r  0) return r; in something that logs errno?

- Not sure whether we need to track semicolon on a variable of its own
  or if we can just check whether *p is the \0 end of the string. I
  noticed the original code had a case logging ... ignoring trailing
  garbage, but I'm not sure where that's needed... Do we need that here
  as well or is that something that will not happen with
  unquote_first_word?

Tested it by running the test suite.

I also built a systemd with this patch, installed and ran it
successfully on a VM running Arch Linux.  I didn't notice anything too
weird (but maybe I just didn't look at all the right places...)

Comments welcome!

Cheers,
Filipe
---
 src/core/load-fragment.c  | 209 ++
 src/test/test-unit-file.c |  14 +++-
 2 files changed, 108 insertions(+), 115 deletions(-)

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index c95c11014c72..dd2cf6e3257f 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -520,9 +520,9 @@ int config_parse_exec(
 void *data,
 void *userdata) {
 
-ExecCommand **e = data, *nce;
-char *path, **n;
-unsigned k;
+ExecCommand **e = data;
+const char *p;
+bool semicolon;
 int r;
 
 assert(filename);
@@ -538,150 +538,137 @@ int config_parse_exec(
 return 0;
 }
 
+rvalue += strspn(rvalue, WHITESPACE);
+p = rvalue;
+
 /* We accept an absolute path as first argument, or
  * alternatively an absolute prefixed with @ to allow
  * overriding of argv[0]. */
-for (;;) {
+do {
 int i;
-const char *word, *state, *reason;
-size_t l;
+_cleanup_strv_free_ char **n = NULL;
+_cleanup_free_ ExecCommand *nce = NULL;
+_cleanup_free_ char *path = NULL;
+_cleanup_free_ char *firstword = NULL;
+char *f;
 bool separate_argv0 = false, ignore = false;
 
-path = NULL;
-nce = NULL;
-n = NULL;
+semicolon = false;
 
-rvalue += strspn(rvalue, WHITESPACE);
+r = unquote_first_word(p, firstword, UNQUOTE_CUNESCAPE);
+if (r  0)
+return r;
+if (r == 0)
+/* We get here in two situations:
+ * 1. If we have only whitespace (shouldn't this reset
+ *the list same as an empty string?)
+ * 2. With a trailing semicolon and no command after 
that.
+ */
+return 0;
 
-if (rvalue[0] == 0)
-break;
+f = firstword;
+for (i = 0; i  2; i++) {
+if (*f == '-'  !ignore) {
+ignore = true;
+f ++;
+} else if (*f == '@'  !separate_argv0) {
+separate_argv0 = true;
+f ++;
+}
+}
 
-k = 0;
-FOREACH_WORD_QUOTED(word, l, rvalue, state) {
-if (k == 0) {
-for (i = 0; i  2; i++) {
-if 

[systemd-devel] [PATCH] build-sys: Normalize paths of configure options

[Filipe Brandenburger]

From: Michael Biebl bi...@debian.org

Strip trailing slashes from options such as --with-rootprefix, so that building
with rootprefix=/ results in paths like /lib instead of //lib.

Also handle paths such as /usr/ gracefully.

Use m4/ax_normalize_path.m4 from the autoconf-archive project, which is now
included in our tree as per usual practices in using autoconf-archive macros.

Tested with the following configure options:
  ./configure \
--with-rootprefix=/ \
--with-rootlibdir=/lib64/ \
--prefix=/usr/ \
--libdir=/lib/ \
--with-bashcompletiondir=/bash-completion/completions/

(The prefix and libdir are already automatically normalized by Autoconf,
this command is testing the others.)

Compared the config.log and resulting trees (in particular man pages) to
confirm double slashes were not present in the latter.

Also tested that a configuration using default options is not affected and that
`make distcheck` still works as expected.
---
 configure.ac|   9 
 m4/ax_normalize_path.m4 | 115 
 2 files changed, 124 insertions(+)
 create mode 100644 m4/ax_normalize_path.m4

diff --git a/configure.ac b/configure.ac
index 92654a6..78d52e4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1370,16 +1370,19 @@ AC_ARG_WITH([dbuspolicydir],
 AS_HELP_STRING([--with-dbuspolicydir=DIR], [D-Bus policy directory]),
 [],
 [with_dbuspolicydir=${sysconfdir}/dbus-1/system.d])
+AX_NORMALIZE_PATH([with_dbuspolicydir])
 
 AC_ARG_WITH([dbussessionservicedir],
 AS_HELP_STRING([--with-dbussessionservicedir=DIR], [D-Bus session 
service directory]),
 [],
 [with_dbussessionservicedir=${datadir}/dbus-1/services])
+AX_NORMALIZE_PATH([with_dbussessionservicedir])
 
 AC_ARG_WITH([dbussystemservicedir],
 AS_HELP_STRING([--with-dbussystemservicedir=DIR], [D-Bus system 
service directory]),
 [],
 [with_dbussystemservicedir=${datadir}/dbus-1/system-services])
+AX_NORMALIZE_PATH([with_dbussystemservicedir])
 
 AC_ARG_WITH([bashcompletiondir],
 AS_HELP_STRING([--with-bashcompletiondir=DIR], [Bash completions 
directory]),
@@ -1389,29 +1392,35 @@ AC_ARG_WITH([bashcompletiondir],
 ] , [
 with_bashcompletiondir=${datadir}/bash-completion/completions
 ])])
+AX_NORMALIZE_PATH([with_bashcompletiondir])
 
 AC_ARG_WITH([zshcompletiondir],
 AS_HELP_STRING([--with-zshcompletiondir=DIR], [Zsh completions 
directory]),
 [], [with_zshcompletiondir=${datadir}/zsh/site-functions])
+AX_NORMALIZE_PATH([with_zshcompletiondir])
 
 AC_ARG_WITH([rootprefix],
 AS_HELP_STRING([--with-rootprefix=DIR], [rootfs directory prefix for 
config files and kernel modules]),
 [], [with_rootprefix=${ac_default_prefix}])
+AX_NORMALIZE_PATH([with_rootprefix])
 
 AC_ARG_WITH([rootlibdir],
 AS_HELP_STRING([--with-rootlibdir=DIR], [Root directory for libraries 
necessary for boot]),
 [],
 [with_rootlibdir=${libdir}])
+AX_NORMALIZE_PATH([with_rootlibdir])
 
 AC_ARG_WITH([pamlibdir],
 AS_HELP_STRING([--with-pamlibdir=DIR], [Directory for PAM modules]),
 [],
 [with_pamlibdir=${with_rootlibdir}/security])
+AX_NORMALIZE_PATH([with_pamlibdir])
 
 AC_ARG_WITH([pamconfdir],
 AS_HELP_STRING([--with-pamconfdir=DIR], [Directory for PAM 
configuration]),
 [],
 [with_pamconfdir=${sysconfdir}/pam.d])
+AX_NORMALIZE_PATH([with_pamconfdir])
 
 AC_ARG_ENABLE([split-usr],
 AS_HELP_STRING([--enable-split-usr], [Assume that /bin, /sbin aren\'t 
symlinks into /usr]),
diff --git a/m4/ax_normalize_path.m4 b/m4/ax_normalize_path.m4
new file mode 100644
index 000..e8f9973
--- /dev/null
+++ b/m4/ax_normalize_path.m4
@@ -0,0 +1,115 @@
+# ===
+# http://www.gnu.org/software/autoconf-archive/ax_normalize_path.html
+# ===
+#
+# SYNOPSIS
+#
+#   AX_NORMALIZE_PATH(VARNAME, [REFERENCE_STRING])
+#
+# DESCRIPTION
+#
+#   Perform some cleanups on the value of $VARNAME (interpreted as a path):
+#
+# - empty paths are changed to '.'
+# - trailing slashes are removed
+# - repeated slashes are squeezed except a leading doubled slash '//'
+#   (which might indicate a networked disk on some OS).
+#
+#   REFERENCE_STRING is used to turn '/' into '\' and vice-versa: if
+#   REFERENCE_STRING contains some backslashes, all slashes and backslashes
+#   are turned into backslashes, otherwise they are all turned into slashes.
+#
+#   This makes processing of DOS filenames quite easier, because you can
+#   turn a filename to the Unix notation, make your processing, and turn it
+#   back to original notation.
+#
+# filename='A:\FOO\\BAR\'
+# old_filename=$filename
+# # Switch to the unix notation
+# AX_NORMALIZE_PATH([filename], [/])
+# 

Re: [systemd-devel] [PATCH 1/2] configure.ac: strip off trailing slashed from $rootprefix

[Filipe Brandenburger]

Hi,

On Fri, May 29, 2015 at 6:04 PM, Michael Biebl mbi...@gmail.com wrote:
 I we assume, autconf-archive is installed, the resulting patch would
 look like the attached diff.
 For convenience sake, we could also ship a copy of
 /usr/share/aclocal/ax_normalize_path.m4 in our m4/ directory.

Yep... indeed autoconf-archive is the way to go.

I tested your patch with some configure options and confirmed it works
as expected. I also checked that it doesn't break distcheck or
anything, so I think it's good to go...

I'll resend it here including the embedded autoconf-archive file in a
format that's ready for git am.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [systemd-commits] Makefile.am

[Filipe Brandenburger]

Hi,

I was thinking about this one recently...

I really think the correct solution is for man/custom-entities.ent
to be generated by configure from a man/custom-entities.ent.in
template instead. I haven't really checked if that's viable though, if
configure knows about every variable it will need, etc. but I was
planning to look into it at some point in the near future.

On Fri, May 29, 2015 at 9:39 AM, Michael Biebl mbi...@gmail.com wrote:
 2015-05-29 15:56 GMT+02:00 Lennart Poettering lenn...@poettering.net:
 I think it's ok to expect people to run make clean after touching
 the makefile to get the man pages fixed.

 Atm, man/custom-entities.ent is only cleaned up on make distclean.
 I think we should move that from DISTCLEANFILES to CLEANFILES.

Yes I think that's a good start and `make clean` will trigger a
rebuild of most of what depends on custom-entities.ent anyways so I
wouldn't expect a lot of spurious rebuilds due to that.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 0/2] Using XML entities for paths in manpages

[Filipe Brandenburger]

Hi Daniel,

I haven't tested it, but I do have a few comments.

First, why not use rootlibdir instead of rootprefixlibdir? There's
already similar rootbindir and rootlibexecdir defined there, so I
think we could stick to the same convention.

From a few lines down in Makefile.am:

  # And these are the special ones for /
  rootprefix=@rootprefix@
  rootbindir=$(rootprefix)/bin
  rootlibexecdir=$(rootprefix)/lib/systemd

On Fri, May 29, 2015 at 2:05 AM, Daniel Mack dan...@zonque.org wrote:
 I had to introduce a new entity in custom-entites.ent, because with
 --with-rootprefix=/, rootprefix;/lib resolves to //lib, and with
 the default behaviour of configure, rootprefix;lib becomes /usrlib.

Debian is already using --with-rootprefix= (an empty string) exactly
for that reason.
http://anonscm.debian.org/cgit/pkg-systemd/systemd.git/tree/debian/rules#n16

If rootprefix=/, there would be also problems with rootbindir=//bin
and other such definitions getting double slashes.

Having said that, I do agree that it would be nice to handle
--with-rootprefix=/ correctly without creating double slashes, but I
think it would be preferrable to handle that in general and not for
the specific case of /lib.

Perhaps just remove any trailing slashes from rootprefix in
configure.ac instead before generating the Makefile? That would also
handle cases like --with-rootprefix=/usr/ to still generate /usr/bin
and /usr/lib correctly.

In any case, handling trailing slashes/double slashes could come on a
separate patch. As pointed out, the problem with it already exists and
Debian is working around it by setting rootprefix to an empty string,
so it's not really strictly required for the rootlibdir patch itself.

I'd be glad to test a new patchset that addresses these suggestions.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/2] configure.ac: strip off trailing slashed from $rootprefix

[Filipe Brandenburger]

On Fri, May 29, 2015 at 5:21 PM, Michael Biebl mbi...@gmail.com wrote:
 autoconf already strips trailing slashes for all default directory
 variables [1].

How does it handle --prefix=/ though? Does it turn it into an empty string?

 I think we should do the same for *all* our custom --with-$foo-dir
 variables, not just rootlibdir.

Agreed.

I think we can go ahead with patch #2 (man: replace hard-coded
/usr/lib) since it works for both the --with-rootprefix=/usr
(default) case and the --with-rootprefix= (empty) case used by Debian.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 0/2] Using XML entities for paths in manpages

[Filipe Brandenburger]

On Thu, May 28, 2015 at 10:44 AM, Michael Biebl mbi...@gmail.com wrote:
 2015-05-28 19:41 GMT+02:00 Martin Pitt martin.p...@ubuntu.com:
 \o/ Many thanks Filipe, that's great! Biggest patch gone :)

 A huge thanks from me as well to everyone involved!

Lennart: Thanks for applying it.

Martin and Michael: You're welcome, glad to help!

We're actually still missing a small part of it (A sentence like
Files in /etc have the highest priority, files in /run take
precedence over files with the same name in */usr/lib*. in files like
hwdb.xml, the last /usr/lib won't get fixed) but it requires new
variables. I'm leaning towards introducing a rootsysconfdir=/etc and
rootlibdir=$(rootprefix)/lib (we already have a rootbindir) so I'll
follow up with a patch doing that.

Though having the first patchset in certainly helps.

Cheers!
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] man: use configured path for mount and umount binaries in manpages

[Filipe Brandenburger]

Export the MOUNT_PATH and UMOUNT_PATH variables as XML entities and use them in
the systemctl.1 manpage instead of hardcoding the path in /usr/bin.

Tested:
- Ran ./configure ac_cv_path_MOUNT_PATH=/bin/mount (same for umount) and
  rebuilt the manpages, confirmed that the correct path was in man/systemctl.1
- Rebuilt man/systemd.directives.xml and the man pages derived from it,
  confirmed that the correct paths were there as well.
---
 Makefile.am   | 2 ++
 man/systemctl.xml | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index d6010c5..98ceb77 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -6505,6 +6505,8 @@ substitutions = \
'|DEBUGTTY=$(DEBUGTTY)|' \
'|KILL=$(KILL)|' \
'|KMOD=$(KMOD)|' \
+   '|MOUNT_PATH=$(MOUNT_PATH)|' \
+   '|UMOUNT_PATH=$(UMOUNT_PATH)|' \
'|MKDIR_P=$(MKDIR_P)|' \
'|QUOTAON=$(QUOTAON)|' \
'|QUOTACHECK=$(QUOTACHECK)|' \
diff --git a/man/systemctl.xml b/man/systemctl.xml
index a2c8a73..35f47de 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -413,8 +413,8 @@
   processes. Not all unit types manage processes of these
   types however. For example, for mount units, control processes
   are defined (which are the invocations of
-  filename/usr/bin/mount/filename and
-  filename/usr/bin/umount/filename), but no main process
+  filenameMOUNT_PATH;/filename and
+  filenameUMOUNT_PATH;/filename), but no main process
   is defined. If omitted, defaults to
   optionall/option./para
 /listitem
-- 
2.4.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] 220 udev boot regression: timeout, giving up waiting for workers to finish

[Filipe Brandenburger]

Hi Tom,

On Wed, May 27, 2015 at 8:45 AM, Tom Gundersen t...@jklm.no wrote:
 It appears a few people see this, but I was not able to reproduce. If
 anyone could reproduce with this patch applied [0], it would be most
 helpful (and post the output of journalctl -b -u systemd-udevd).

Done.

Console output from udev in initramfs:
http://paste.fedoraproject.org/226145/43216143/

And the output of the journalctl command you asked:
http://paste.fedoraproject.org/226146/43274324/

I have this on Arch Linux with mkinitcpio. Using latest systemd from
git plus your patch.

HTH!
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 0/2] Using XML entities for paths in manpages

[Filipe Brandenburger]

Hi,

On Wed, May 27, 2015 at 6:04 AM, Lennart Poettering
lenn...@poettering.net wrote:
 Hmm, any chance we can somehow define those entities without having to
 add

  !ENTITY % entities SYSTEM custom-entities.ent 
  %entities;
  ]

 To each file? Can't we tell xsltproc about this via some command line
 switch or so?

I don't think that's possible, in part I believe because XML wants the
documents to be completely described by their doctype and having
external magic includes would break that promise.

In particular, two man pages were using this exact same snippet before
my patch: systemd.generator.xml and systemd.unit.xml (though the
latter didn't seem to use any of the named entities at that point...)

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] 220 udev boot regression: timeout, giving up waiting for workers to finish

[Filipe Brandenburger]

On Wed, May 27, 2015 at 10:08 AM, Tom Gundersen t...@jklm.no wrote:
 This should be fixed by 86c3bece38bcf55da6387d20c6f01da9ad0284dc.
 Thanks for the help in debugging this, and sorry for the
 inconvenience.

And I can confirm the timeout is gone.

Thanks for fixing it quickly Tom! Glad to help you isolate it.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 4/4] cgtop: support time header with user-specified format string

[Filipe Brandenburger]

Hi,

On Wed, May 27, 2015 at 3:40 PM, Charles Duffy char...@dyfis.net wrote:
 +#pragma GCC diagnostic push
 +#pragma GCC diagnostic ignored -Wformat-nonliteral
 +time_len = strftime(buf, l, arg_time_format, curr_time);
 +if (time_len = 0)
 +return;
 +#pragma GCC diagnostic pop

You could instead use gcc's __attribute__ ((format (strftime, ...)))
to indicate that arg_time_format is supposed to be a valid strftime
format.

https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-g_t_0040code_007bflatten_007d-function-attribute-3080

See also the definition of _printf_ in src/shared/macro.h and its uses.

Though I'm not sure whether taking a strftime format as a command line
argument is really a good idea... But I'll defer that to other
reviewers.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Reduce unit-loading time

[Filipe Brandenburger]

Hi,

On Sun, May 24, 2015 at 8:41 PM, cee1 fykc...@gmail.com wrote:
 I tried ureadahead, but got following error:

 write(2, ureadahead: Error while tracing:..., 59ureadahead: Error
 while tracing: No such file or directory

 Needs an out-of-tree kernel patch?

Yes, ureadahead needs an out-of-tree kernel patch to add trace events
for open(), exec() and uselib() syscalls that take file paths.

http://bugs.launchpad.net/bugs/462111

AFAICT, this never went upstream because upstream was discussing a
generic approach of tracing any system calls, but I believe that never
really panned out...

HTH,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] udev now crashes in daemon mode

[Filipe Brandenburger]

Hi Tom,

On Sun, May 24, 2015 at 6:30 AM, Tom Gundersen t...@jklm.no wrote:
 On Sun, May 24, 2015 at 11:40 AM, Mantas Mikulėnas graw...@gmail.com wrote:
 So, udev v220 crashes in my initramfs with the following message:

 starting version v220
 Assertion 'manager-pid == getpid()' failed at src/udev/udevd.c:568,
 function ev
 Aborting.

 It seems main calls manager_new() before forking, so the parent PID is
 stored instead of child PID.

 (I'm using Arch Linux with the traditional mkinitcpio-based initramfs, which
 starts udev using systemd-udevd --daemon --resolve-names=never.)

 Thanks for the report. This should be fixed now in git, please let me
 know if that is not the case.

I was testing the same setup (ArchLinux with mkinitcpio), seeing the
same crash in udevd from initramfs and I can confirm that
040e689654ef08 (udevd: event - fix event queue in daemenozied mode)
fixed it. Thanks!

HTH,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/3] sd-daemon: Use LISTEN_NAMES env when available

[Filipe Brandenburger]

Hi,

On Mon, May 18, 2015 at 7:26 AM, Krzysztof Opasiak
k.opas...@samsung.com wrote:
 Matching between fds and list of expected paths is done in n^2

I don't think that's the case, because you can just stat() all the
names and fstat() all the fds, then sort both lists on inode numbers
and then traverse both in order matching inode orders on each list, so
it's actually O(n * log n).

Not that it matters that much, I don't expect this scheme to be used
for a very large number of fds/labels...

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] machine: add reference to machine-dbus.h to Makefile.am

[Filipe Brandenburger]

Ping?

distcheck is still failing due to the lack of reference to this header
file in Makefile.am

Cheers,
Filipe


On Mon, Dec 29, 2014 at 3:22 PM, Filipe Brandenburger
filbran...@google.com wrote:
 Commit 003dffde2c1b93 (machined: Move image discovery logic into src/shared,
 so that we can make use of it from nspawn) moved some definitions from
 machine.h to a new machine-dbus.h, but did not include it in Makefile.am

 Tested that `make distcheck` works after this fix.
 ---
  Makefile.am | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/Makefile.am b/Makefile.am
 index 28d2e4b..8cdf1db 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -5044,6 +5044,7 @@ libsystemd_machine_core_la_SOURCES = \
 src/machine/machine.h \
 src/machine/machined-dbus.c \
 src/machine/machine-dbus.c \
 +   src/machine/machine-dbus.h \
 src/machine/image-dbus.c \
 src/machine/image-dbus.h

 --
 1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] units: move machines.target to dist_systemunit_DATA

[Filipe Brandenburger]

Move units/machines.target from nodist_systemunit_DATA to dist_systemunit_DATA,
since it's not a generated file. Otherwise, `make clean` would remove the
committed copy of the file.

Tested that `./autogen.sh c` will not remove it and that `make distcheck` works
after this fix.
---
 Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 8cdf1db..c12f2f6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -506,6 +506,7 @@ dist_systemunit_DATA = \
units/systemd-udevd-control.socket \
units/systemd-udevd-kernel.socket \
units/system-update.target \
+   units/machines.target \
units/initrd-switch-root.target
 
 if ENABLE_KDBUS
@@ -549,7 +550,6 @@ nodist_systemunit_DATA = \
units/initrd-udevadm-cleanup-db.service \
units/initrd-switch-root.service \
units/systemd-nspawn@.service \
-   units/machines.target \
units/systemd-update-done.service
 
 if HAVE_UTMP
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] machine: add reference to machine-dbus.h to Makefile.am

[Filipe Brandenburger]

Commit 003dffde2c1b93 (machined: Move image discovery logic into src/shared,
so that we can make use of it from nspawn) moved some definitions from
machine.h to a new machine-dbus.h, but did not include it in Makefile.am

Tested that `make distcheck` works after this fix.
---
 Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile.am b/Makefile.am
index 28d2e4b..8cdf1db 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5044,6 +5044,7 @@ libsystemd_machine_core_la_SOURCES = \
src/machine/machine.h \
src/machine/machined-dbus.c \
src/machine/machine-dbus.c \
+   src/machine/machine-dbus.h \
src/machine/image-dbus.c \
src/machine/image-dbus.h
 
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/2] test: only use assert_se in test_raw_clone

[Filipe Brandenburger]

The asserts used in the tests should never be allowed to be optimized away.
---
 src/test/test-util.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/test/test-util.c b/src/test/test-util.c
index 222af9a..57fd19b 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -420,7 +420,7 @@ static void check(const char *test, char** expected, bool 
trailing) {
 printf(%s\n, t);
 }
 printf(%s\n, state);
-assert(expected[i] == NULL);
+assert_se(expected[i] == NULL);
 assert_se(isempty(state) == !trailing);
 }
 
@@ -1331,15 +1331,15 @@ static void test_raw_clone(void) {
 assert_se(raw_getpid() == parent);
 
 pid = raw_clone(0, NULL);
-assert(pid = 0);
+assert_se(pid = 0);
 
 pid2 = raw_getpid();
 log_info(raw_clone: PID_FMT getpid()→PID_FMT 
raw_getpid()→PID_FMT,
  pid, getpid(), pid2);
 if (pid == 0)
-assert(pid2 != parent);
+assert_se(pid2 != parent);
 else
-assert(pid2 == parent);
+assert_se(pid2 == parent);
 }
 
 int main(int argc, char *argv[]) {
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] test: wait for cloned thread to exit

[Filipe Brandenburger]

In test_raw_clone, make sure the cloned thread calls _exit() and in the parent
thread call waitpid(..., __WCLONE) to wait for the child thread to terminate,
otherwise there is a race condition where the child thread will log to the
console after the test process has already exited and the assertion from the
child thread might not be enforced.

The absence of this patch might also create problems for other tests that would
be added after this one, since potentially both parent and child would run
those tests as the child would continue running.

Tested by confirming that the logs from the child are printed before the test
terminates and that a false assertion in the child aborts the test with a core
dump.
---
 src/test/test-util.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/test/test-util.c b/src/test/test-util.c
index 57fd19b..f88ef2e 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -1336,10 +1336,15 @@ static void test_raw_clone(void) {
 pid2 = raw_getpid();
 log_info(raw_clone: PID_FMT getpid()→PID_FMT 
raw_getpid()→PID_FMT,
  pid, getpid(), pid2);
-if (pid == 0)
+if (pid == 0) {
 assert_se(pid2 != parent);
-else
+_exit(EXIT_SUCCESS);
+} else {
+int status;
 assert_se(pid2 == parent);
+
+waitpid(pid, status, __WCLONE);
+}
 }
 
 int main(int argc, char *argv[]) {
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/2] test: wait for cloned thread to exit

[Filipe Brandenburger]

Hi,

On Tue, Dec 23, 2014 at 6:43 AM, Lennart Poettering
lenn...@poettering.net wrote:
 On Mon, 22.12.14 11:57, Filipe Brandenburger (filbran...@google.com) wrote:
 Ping?

 I got none of these emails, and they are neither shown in the mailing
 list archives. There must be something wrong in the mail delivery
 between google.com and fdo?

Indeed, that seemed to be the case...

I set up a different smtp server and resent, from the mailing list
archives looks like it was delivered this time:
http://lists.freedesktop.org/archives/systemd-devel/2014-December/026521.html

I'll rebase and resend all others.

Thanks!
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 0/9] capabilities: remove include of sys/capability.h where possible

[Filipe Brandenburger]

This is a first cleanup step towards removing the dependency on libcap.

The idea of removing the libcap dependency was brought up by Lennart in:
http://lists.freedesktop.org/archives/systemd-devel/2014-December/026155.html

It is mainly removing the include of sys/capability.h where the only
capability-related information used is the CAP_* constants which are actually
coming from linux/capability.h (kernel headers) or from missing.h (for
compatibility with older kernel headers.)

Filipe Brandenburger (9):
  capabilities: remove spurious include of sys/capability.h from nspawn.c
  capabilities: remove spurious include of sys/capability.h from logind 
sources
  capabilities: remove spurious include of sys/capability.h from tmpfiles.c
  capabilities: remove spurious include of sys/capability.h from hostnamed.c
  capabilities: remove spurious include of sys/capability.h from localed.c
  capabilities: remove spurious include of sys/capability.h from timedated.c
  capabilities: remove spurious include of sys/capability.h from pam_systemd.c
  capabilities: remove spurious include of sys/capability.h from machined 
sources
  capabilities: remove spurious include of sys/capability.h from sd-dbus 
sources

 src/hostname/hostnamed.c| 1 -
 src/libsystemd/sd-bus/bus-objects.c | 2 --
 src/libsystemd/sd-bus/bus-util.c| 1 -
 src/locale/localed.c| 1 -
 src/login/logind-dbus.c | 1 -
 src/login/logind-seat-dbus.c| 1 -
 src/login/logind-session-dbus.c | 1 -
 src/login/logind-user-dbus.c| 1 -
 src/login/pam_systemd.c | 1 -
 src/machine/machine-dbus.c  | 1 -
 src/machine/machined-dbus.c | 1 -
 src/nspawn/nspawn.c | 2 +-
 src/timedate/timedated.c| 1 -
 src/tmpfiles/tmpfiles.c | 1 -
 14 files changed, 1 insertion(+), 15 deletions(-)

-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 4/9] capabilities: remove spurious include of sys/capability.h from hostnamed.c

[Filipe Brandenburger]

It does not use any functions from libcap directly. The CAP_SYS_ADMIN constant
in use by this file comes from linux/capability.h imported through 
missing.h.

Tested that systemd-hostnamed builds cleanly and works after this change.
---
 src/hostname/hostnamed.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c
index ef45e56..b230ff6 100644
--- a/src/hostname/hostnamed.c
+++ b/src/hostname/hostnamed.c
@@ -23,7 +23,6 @@
 #include string.h
 #include unistd.h
 #include sys/utsname.h
-#include sys/capability.h
 
 #include util.h
 #include strv.h
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 3/9] capabilities: remove spurious include of sys/capability.h from tmpfiles.c

[Filipe Brandenburger]

It does not use any functions from libcap directly. The CAP_MKNOD constant in
use by this file comes from linux/capability.h imported through missing.h.

Tested that systemd-tmpfiles builds cleanly and works after this change.
---
 src/tmpfiles/tmpfiles.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index d40bd96..44ea51e 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -38,7 +38,6 @@
 #include sys/param.h
 #include glob.h
 #include fnmatch.h
-#include sys/capability.h
 #include sys/xattr.h
 
 #include log.h
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 7/9] capabilities: remove spurious include of sys/capability.h from pam_systemd.c

[Filipe Brandenburger]

It does not use any functions or constants from libcap directly.

Tested that pam_systemd.la builds cleanly and works after this change.
---
 src/login/pam_systemd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c
index 111e2b7..d5b29c8 100644
--- a/src/login/pam_systemd.c
+++ b/src/login/pam_systemd.c
@@ -24,7 +24,6 @@
 #include sys/file.h
 #include pwd.h
 #include endian.h
-#include sys/capability.h
 
 #include security/pam_modules.h
 #include security/_pam_macros.h
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 6/9] capabilities: remove spurious include of sys/capability.h from timedated.c

[Filipe Brandenburger]

It does not use any functions from libcap directly. The CAP_SYS_TIME constant
in use by this file comes from linux/capability.h imported through 
missing.h.

Tested that systemd-timedated builds cleanly and works after this change.
---
 src/timedate/timedated.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index bf567a1..d507200 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -22,7 +22,6 @@
 #include errno.h
 #include string.h
 #include unistd.h
-#include sys/capability.h
 
 #include sd-id128.h
 #include sd-messages.h
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/9] capabilities: remove spurious include of sys/capability.h from nspawn.c

[Filipe Brandenburger]

It does not use any functions from libcap directly. The CAP_* constants in use
through this file come from missing.h which will import linux/capability.h
and complement it with CAP_* constants not defined by the current kernel
headers.

Add an explicit import of our capability.h since it does use the function
capability_bounding_set_drop from that header file. Previously, that header was
implicitly imported through through cap-list.h.

Tested that systemd-nspawn builds cleanly and works after this change.
---
 src/nspawn/nspawn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 0dd12ad..04396eb 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -31,7 +31,6 @@
 #include stdio.h
 #include errno.h
 #include sys/prctl.h
-#include sys/capability.h
 #include getopt.h
 #include termios.h
 #include sys/signalfd.h
@@ -90,6 +89,7 @@
 #include base-filesystem.h
 #include barrier.h
 #include event-util.h
+#include capability.h
 #include cap-list.h
 #include btrfs-util.h
 
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 8/9] capabilities: remove spurious include of sys/capability.h from machined sources

[Filipe Brandenburger]

They do not use any functions from libcap directly. The CAP_KILL constant in
use by these files comes from linux/capability.h imported through
missing.h.

Tested that systemd-machined builds cleanly and works after this change.
---
 src/machine/machine-dbus.c  | 1 -
 src/machine/machined-dbus.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c
index 76c5dcf..7855c4c 100644
--- a/src/machine/machine-dbus.c
+++ b/src/machine/machine-dbus.c
@@ -21,7 +21,6 @@
 
 #include errno.h
 #include string.h
-#include sys/capability.h
 #include arpa/inet.h
 
 #include bus-util.h
diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c
index 370d04a..ffb7722 100644
--- a/src/machine/machined-dbus.c
+++ b/src/machine/machined-dbus.c
@@ -23,7 +23,6 @@
 #include string.h
 #include unistd.h
 #include pwd.h
-#include sys/capability.h
 
 #include sd-id128.h
 #include sd-messages.h
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 5/9] capabilities: remove spurious include of sys/capability.h from localed.c

[Filipe Brandenburger]

It does not use any functions from libcap directly. The CAP_SYS_ADMIN constant
in use by this file comes from linux/capability.h imported through 
missing.h.

Tested that systemd-localed builds cleanly and works after this change.
---
 src/locale/localed.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/locale/localed.c b/src/locale/localed.c
index 0aaa63d..0723541 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -23,7 +23,6 @@
 #include errno.h
 #include string.h
 #include unistd.h
-#include sys/capability.h
 
 #include sd-bus.h
 
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 9/9] capabilities: remove spurious include of sys/capability.h from sd-dbus sources

[Filipe Brandenburger]

They do not use any functions from libcap directly. The CAP_SYS_ADMIN constant
in use by bus-objects.c comes from linux/capability.h imported through
missing.h. The missing.h header is imported through util.h which gets
imported in bus-util.h.

Tested that everything builds cleanly after this change.
---
 src/libsystemd/sd-bus/bus-objects.c | 2 --
 src/libsystemd/sd-bus/bus-util.c| 1 -
 2 files changed, 3 deletions(-)

diff --git a/src/libsystemd/sd-bus/bus-objects.c 
b/src/libsystemd/sd-bus/bus-objects.c
index 6162d12..e64743f 100644
--- a/src/libsystemd/sd-bus/bus-objects.c
+++ b/src/libsystemd/sd-bus/bus-objects.c
@@ -19,8 +19,6 @@
   along with systemd; If not, see http://www.gnu.org/licenses/.
 ***/
 
-#include sys/capability.h
-
 #include strv.h
 #include set.h
 #include bus-internal.h
diff --git a/src/libsystemd/sd-bus/bus-util.c b/src/libsystemd/sd-bus/bus-util.c
index 0f1a89c..06e6d84 100644
--- a/src/libsystemd/sd-bus/bus-util.c
+++ b/src/libsystemd/sd-bus/bus-util.c
@@ -20,7 +20,6 @@
 ***/
 
 #include sys/socket.h
-#include sys/capability.h
 
 #include systemd/sd-daemon.h
 
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 0/9] capabilities: remove include of sys/capability.h where possible

[Filipe Brandenburger]

On Tue, Dec 23, 2014 at 5:23 AM, David Herrmann dh.herrm...@gmail.com wrote:
 I cannot find these patches on systemd-devel@lists.freedesktop.org.
 This might be due to fdo mail-server issues, or me just being
 incapable of searching through my emails... Anyway, would you mind
 resending those?

Yeah, it seems I was having trouble... Looks like I fixed it, at least
I can see the messages I just resent on the mailing list archive.

 While at it, they look like you can merge all this into a single patch.

I see. The main reason for it was that I used the commit description
to document that it is safe to remove from every place. If you'd
really like I can squash them all myself or feel free to squash them
while applying, that's certainly fine with me.

Thanks!
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCHv2] update .gitignore to include test-lldp

[Filipe Brandenburger]

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 1b5d60f..078fd9a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -207,6 +207,7 @@
 /test-libudev-sym*
 /test-list
 /test-unaligned
+/test-lldp
 /test-locale-util
 /test-local-addresses
 /test-log
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCHv2 1/2] build-sys: do not use pkgconfig dbus-1.pc to find dbus directories

[Filipe Brandenburger]

Do not use the dbus-1.pc pkgconfig settings to determine dbus directories. Use
directories relative to ${sysconfdir} and ${datadir} instead.

This approach was suggested by Simon McVittie in:
http://lists.freedesktop.org/archives/systemd-devel/2014-October/024388.html

Tested by building and installing systemd without the dbus-devel installed.
Without this patch, the dbus files and directories end up in the root of the
filesystem. With this patch, they end up in the same locations as previously
(assuming default ${sysconfdir} and ${datadir}) whether dbus-devel is present
or not. Also made sure that `make check` works without dbus-devel installed.
---
v2: Updated TODO to remove entry about pkg-config dbus-1.

 TODO | 2 --
 configure.ac | 8 
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/TODO b/TODO
index fdc37b1..6ff9b8f 100644
--- a/TODO
+++ b/TODO
@@ -126,8 +126,6 @@ Features:
 
 * systemd --user should issue sd_notify() upon reaching basic.target, not on 
becoming idle
 
-* configure.ac pretends dbus was optional but actually hardcodes use of dbus' 
pkg-config file to determine various dbus dirs such as policy and activation 
dirs
-
 * consider showing the unit names during boot up in the status output, not 
just the unit descriptions
 
 * dhcp: do we allow configuring dhcp routes on interfaces that are not the one 
we got the dhcp info from?
diff --git a/configure.ac b/configure.ac
index 0e72166..f20c0e7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1258,22 +1258,22 @@ AC_SUBST(TTY_GID)
 AC_ARG_WITH([dbuspolicydir],
 AS_HELP_STRING([--with-dbuspolicydir=DIR], [D-Bus policy directory]),
 [],
-[with_dbuspolicydir=$($PKG_CONFIG --variable=sysconfdir 
dbus-1)/dbus-1/system.d])
+[with_dbuspolicydir=${sysconfdir}/dbus-1/system.d])
 
 AC_ARG_WITH([dbussessionservicedir],
 AS_HELP_STRING([--with-dbussessionservicedir=DIR], [D-Bus session 
service directory]),
 [],
-[with_dbussessionservicedir=$($PKG_CONFIG 
--variable=session_bus_services_dir dbus-1)])
+[with_dbussessionservicedir=${datadir}/dbus-1/services])
 
 AC_ARG_WITH([dbussystemservicedir],
 AS_HELP_STRING([--with-dbussystemservicedir=DIR], [D-Bus system 
service directory]),
 [],
-[with_dbussystemservicedir=$(readlink -m $($PKG_CONFIG 
--variable=session_bus_services_dir dbus-1)/../system-services)])
+[with_dbussystemservicedir=${datadir}/dbus-1/system-services])
 
 AC_ARG_WITH([dbusinterfacedir],
 AS_HELP_STRING([--with-dbusinterfacedir=DIR], [D-Bus interface 
directory]),
 [],
-[with_dbusinterfacedir=$(readlink -m $($PKG_CONFIG 
--variable=session_bus_services_dir dbus-1)/../interfaces)])
+[with_dbusinterfacedir=${datadir}/dbus-1/interfaces])
 
 AC_ARG_WITH([bashcompletiondir],
 AS_HELP_STRING([--with-bashcompletiondir=DIR], [Bash completions 
directory]),
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCHv2 2/2] build-sys: remove references to dbusinterfacedir

[Filipe Brandenburger]

This directory is not used by systemd.

Tested by running a full build, running `make install` and comparing the file
list in the target trees and making sure that `make distcheck` still works.
---
 configure.ac | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index f20c0e7..2348dac 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1270,11 +1270,6 @@ AC_ARG_WITH([dbussystemservicedir],
 [],
 [with_dbussystemservicedir=${datadir}/dbus-1/system-services])
 
-AC_ARG_WITH([dbusinterfacedir],
-AS_HELP_STRING([--with-dbusinterfacedir=DIR], [D-Bus interface 
directory]),
-[],
-[with_dbusinterfacedir=${datadir}/dbus-1/interfaces])
-
 AC_ARG_WITH([bashcompletiondir],
 AS_HELP_STRING([--with-bashcompletiondir=DIR], [Bash completions 
directory]),
 [],
@@ -1373,7 +1368,6 @@ test -z $enable_debug  enable_debug=none
 AC_SUBST([dbuspolicydir], [$with_dbuspolicydir])
 AC_SUBST([dbussessionservicedir], [$with_dbussessionservicedir])
 AC_SUBST([dbussystemservicedir], [$with_dbussystemservicedir])
-AC_SUBST([dbusinterfacedir], [$with_dbusinterfacedir])
 AC_SUBST([bashcompletiondir], [$with_bashcompletiondir])
 AC_SUBST([zshcompletiondir], [$with_zshcompletiondir])
 AC_SUBST([pamlibdir], [$with_pamlibdir])
@@ -1476,7 +1470,6 @@ AC_MSG_RESULT([
 D-Bus policy dir:${with_dbuspolicydir}
 D-Bus session dir:   ${with_dbussessionservicedir}
 D-Bus system dir:${with_dbussystemservicedir}
-D-Bus interfaces dir:${with_dbusinterfacedir}
 Bash completions dir:${with_bashcompletiondir}
 Zsh completions dir: ${with_zshcompletiondir}
 Extra start script:  ${RC_LOCAL_SCRIPT_PATH_START}
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [systemd-commits] 8 commits - Makefile.am TODO src/bus-proxyd src/core src/import src/libsystemd src/machine src/shared src/test

[Filipe Brandenburger]

Hi,

On Tue, Dec 23, 2014 at 10:15 AM, Lennart Poettering
lenn...@kemper.freedesktop.org wrote:
 commit 3c70e3bb022f0de3317f3600c9366a2f4597339e
 Author: Lennart Poettering lenn...@poettering.net
 Date:   Tue Dec 23 18:36:04 2014 +0100

 core: rearrange code so that libsystemd/sd-bus/ does not include header 
 files from core

 Stuff in src/shared or src/libsystemd should *never* include code from
 src/core or any of the tools, so don't do that here either. It's not OK!

This commit broke the build, looks like src/core/bus-policy.[ch]
should have been added to this commit but they were not...

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCHv2] test: do not use last cap from kernel in test-cap-list

[Filipe Brandenburger]

The new test-cap-list introduced in commit 2822da4fb7f891 uses the included
table of capabilities. However, it uses cap_last_cap() which probes the kernel
for the last available capability. On an older kernel (e.g. 3.10 from RHEL 7)
that causes the test to fail with the following message:

Assertion '!capability_to_name(cap_last_cap()+1)' failed at 
src/test/test-cap-list.c:30, function main(). Aborting.

Fix it by exporting the size of the static table and using it in the test
instead of the dynamic one from the current kernel.

Tested by successfully running ./test-cap-list and the whole `make check` test
suite with this patch on a RHEL 7 host.
---
v2: Updated the patch to also consider the changes introduced in commit
4b7c1d5d6a0060 (test-cap-list: always check libcap comes to the same names...)

 src/shared/cap-list.c| 4 
 src/shared/cap-list.h| 1 +
 src/test/test-cap-list.c | 7 ---
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/shared/cap-list.c b/src/shared/cap-list.c
index 56d1488..8033e8c 100644
--- a/src/shared/cap-list.c
+++ b/src/shared/cap-list.c
@@ -60,3 +60,7 @@ int capability_from_name(const char *name) {
 
 return sc-id;
 }
+
+int capability_list_length(void) {
+return (int) ELEMENTSOF(capability_names);
+}
diff --git a/src/shared/cap-list.h b/src/shared/cap-list.h
index c699e46..9824fad 100644
--- a/src/shared/cap-list.h
+++ b/src/shared/cap-list.h
@@ -23,3 +23,4 @@
 
 const char *capability_to_name(int id);
 int capability_from_name(const char *name);
+int capability_list_length(void);
diff --git a/src/test/test-cap-list.c b/src/test/test-cap-list.c
index 7c5ae18..4e75136 100644
--- a/src/test/test-cap-list.c
+++ b/src/test/test-cap-list.c
@@ -19,6 +19,7 @@
   along with systemd; If not, see http://www.gnu.org/licenses/.
 ***/
 
+#include util.h
 #include log.h
 #include cap-list.h
 #include capability.h
@@ -27,9 +28,9 @@ int main(int argc, char *argv[]) {
 int i;
 
 assert_se(!capability_to_name(-1));
-assert_se(!capability_to_name(cap_last_cap()+1));
+assert_se(!capability_to_name(capability_list_length()));
 
-for (i = 0; i = (int) cap_last_cap(); i++) {
+for (i = 0; i  capability_list_length(); i++) {
 const char *n;
 
 assert_se(n = capability_to_name(i));
@@ -45,7 +46,7 @@ int main(int argc, char *argv[]) {
 assert_se(capability_from_name(15) == 15);
 assert_se(capability_from_name(-1) == -EINVAL);
 
-for (i = 0; i = (int) cap_last_cap(); i++) {
+for (i = 0; i  capability_list_length(); i++) {
 _cleanup_cap_free_charp_ char *a = NULL;
 const char *b;
 unsigned u;
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/3] machined: add org.freedesktop.machine1.policy.in to POTFILES.in

[Filipe Brandenburger]

The new polkit file was introduced in commit d04c1fb8e21560 (machined:
introduce polkit for OpenLogin() call).
---
 po/POTFILES.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 2829c87..344c307 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -1,5 +1,6 @@
 src/hostname/org.freedesktop.hostname1.policy.in
 src/locale/org.freedesktop.locale1.policy.in
 src/login/org.freedesktop.login1.policy.in
+src/machine/org.freedesktop.machine1.policy.in
 src/timedate/org.freedesktop.timedate1.policy.in
 src/core/org.freedesktop.systemd1.policy.in.in
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/3] build-sys: update path in reference to sd-lldp.h

[Filipe Brandenburger]

The file was moved from src/libsystemd-network to src/systemd in commit
7a6f1457462840 (sd-lldp: minor header cleanup).

This fixes make distcheck.
---
 Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 4173147..8446469 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3054,6 +3054,7 @@ libsystemd_network_la_SOURCES = \
src/systemd/sd-dhcp6-client.h \
src/systemd/sd-dhcp6-lease.h \
src/systemd/sd-pppoe.h \
+   src/systemd/sd-lldp.h \
src/libsystemd-network/sd-dhcp-client.c \
src/libsystemd-network/sd-dhcp-server.c \
src/libsystemd-network/dhcp-network.c \
@@ -3089,7 +3090,6 @@ libsystemd_network_la_SOURCES = \
src/libsystemd-network/lldp-internal.h \
src/libsystemd-network/lldp-internal.c \
src/libsystemd-network/lldp-util.h \
-   src/libsystemd-network/sd-lldp.h \
src/libsystemd-network/sd-lldp.c
 
 libsystemd_network_la_LIBADD = \
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/2] build-sys: do not use pkgconfig dbus-1.pc to find dbus directories

[Filipe Brandenburger]

Ping?

This was discussed before and I think it's the one thing we need to
remove the dependencies on dbus-devel...

(See also patch 2/2 of this series which gets rid of
--dbusinterfacedir which is not used anywhere. We might also want to
change detection of dbus libs in configure.ac but I'd say we start
with these and go from here.)

Cheers,
Filipe

On Fri, Dec 12, 2014 at 4:55 PM, Filipe Brandenburger
filbran...@google.com wrote:
 Do not use the dbus-1.pc pkgconfig settings to determine dbus directories. Use
 directories relative to ${sysconfdir} and ${datadir} instead.

 This approach was suggested by Simon McVittie in:
 http://lists.freedesktop.org/archives/systemd-devel/2014-October/024388.html

 Tested by building and installing systemd without the dbus-devel installed.
 Without this patch, the dbus files and directories end up in the root of the
 filesystem. With this patch, they end up in the same locations as previously
 (assuming default ${sysconfdir} and ${datadir}) whether dbus-devel is present
 or not. Also made sure that `make check` works without dbus-devel installed.
 ---
  configure.ac | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

 diff --git a/configure.ac b/configure.ac
 index e14f3cf..cf9925e 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -1253,22 +1253,22 @@ AC_SUBST(TTY_GID)
  AC_ARG_WITH([dbuspolicydir],
  AS_HELP_STRING([--with-dbuspolicydir=DIR], [D-Bus policy directory]),
  [],
 -[with_dbuspolicydir=$($PKG_CONFIG --variable=sysconfdir 
 dbus-1)/dbus-1/system.d])
 +[with_dbuspolicydir=${sysconfdir}/dbus-1/system.d])

  AC_ARG_WITH([dbussessionservicedir],
  AS_HELP_STRING([--with-dbussessionservicedir=DIR], [D-Bus session 
 service directory]),
  [],
 -[with_dbussessionservicedir=$($PKG_CONFIG 
 --variable=session_bus_services_dir dbus-1)])
 +[with_dbussessionservicedir=${datadir}/dbus-1/services])

  AC_ARG_WITH([dbussystemservicedir],
  AS_HELP_STRING([--with-dbussystemservicedir=DIR], [D-Bus system 
 service directory]),
  [],
 -[with_dbussystemservicedir=$(readlink -m $($PKG_CONFIG 
 --variable=session_bus_services_dir dbus-1)/../system-services)])
 +[with_dbussystemservicedir=${datadir}/dbus-1/system-services])

  AC_ARG_WITH([dbusinterfacedir],
  AS_HELP_STRING([--with-dbusinterfacedir=DIR], [D-Bus interface 
 directory]),
  [],
 -[with_dbusinterfacedir=$(readlink -m $($PKG_CONFIG 
 --variable=session_bus_services_dir dbus-1)/../interfaces)])
 +[with_dbusinterfacedir=${datadir}/dbus-1/interfaces])

  AC_ARG_WITH([bashcompletiondir],
  AS_HELP_STRING([--with-bashcompletiondir=DIR], [Bash completions 
 directory]),
 --
 1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 0/9] capabilities: remove include of sys/capability.h where possible

[Filipe Brandenburger]

Ping?

Also wondering if it makes sense to go ahead and implement our own
cap_to_text and cap_from_text to generate capability strings from
the bitmaps (and further remove dependency on libcap.) I think it
does, considering we now already have our own list of valid
capabilities and the constants come from kernel headers (or
missing.h), it makes sense to have more of our own routines...

Cheers,
Filipe


On Tue, Dec 16, 2014 at 5:18 PM, Filipe Brandenburger
filbran...@google.com wrote:
 This is a first cleanup step towards removing the dependency on libcap.

 The idea of removing the libcap dependency was brought up by Lennart in:
 http://lists.freedesktop.org/archives/systemd-devel/2014-December/026155.html

 It is mainly removing the include of sys/capability.h where the only
 capability-related information used is the CAP_* constants which are actually
 coming from linux/capability.h (kernel headers) or from missing.h (for
 compatibility with older kernel headers.)

 Filipe Brandenburger (9):
   capabilities: remove spurious include of sys/capability.h from nspawn.c
   capabilities: remove spurious include of sys/capability.h from logind 
 sources
   capabilities: remove spurious include of sys/capability.h from tmpfiles.c
   capabilities: remove spurious include of sys/capability.h from hostnamed.c
   capabilities: remove spurious include of sys/capability.h from localed.c
   capabilities: remove spurious include of sys/capability.h from timedated.c
   capabilities: remove spurious include of sys/capability.h from 
 pam_systemd.c
   capabilities: remove spurious include of sys/capability.h from machined 
 sources
   capabilities: remove spurious include of sys/capability.h from sd-dbus 
 sources

  src/hostname/hostnamed.c| 1 -
  src/libsystemd/sd-bus/bus-objects.c | 2 --
  src/libsystemd/sd-bus/bus-util.c| 1 -
  src/locale/localed.c| 1 -
  src/login/logind-dbus.c | 1 -
  src/login/logind-seat-dbus.c| 1 -
  src/login/logind-session-dbus.c | 1 -
  src/login/logind-user-dbus.c| 1 -
  src/login/pam_systemd.c | 1 -
  src/machine/machine-dbus.c  | 1 -
  src/machine/machined-dbus.c | 1 -
  src/nspawn/nspawn.c | 2 +-
  src/timedate/timedated.c| 1 -
  src/tmpfiles/tmpfiles.c | 1 -
  14 files changed, 1 insertion(+), 15 deletions(-)

 --
 1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] update .gitignore to include test-lldp

[Filipe Brandenburger]

Ping?

On Fri, Dec 19, 2014 at 9:45 AM, Filipe Brandenburger
filbran...@google.com wrote:
 ---
  .gitignore | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/.gitignore b/.gitignore
 index 1b5d60f..078fd9a 100644
 --- a/.gitignore
 +++ b/.gitignore
 @@ -207,6 +207,7 @@
  /test-libudev-sym*
  /test-list
  /test-unaligned
 +/test-lldp
  /test-locale-util
  /test-local-addresses
  /test-log
 --
 1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCHv2] test: do not use last cap from kernel in test-cap-list

[Filipe Brandenburger]

Ping?

On Fri, Dec 12, 2014 at 10:47 AM, Filipe Brandenburger
filbran...@google.com wrote:
 The new test-cap-list introduced in commit 2822da4fb7f891 uses the included
 table of capabilities. However, it uses cap_last_cap() which probes the kernel
 for the last available capability. On an older kernel (e.g. 3.10 from RHEL 7)
 that causes the test to fail with the following message:

 Assertion '!capability_to_name(cap_last_cap()+1)' failed at 
 src/test/test-cap-list.c:30, function main(). Aborting.

 Fix it by exporting the size of the static table and using it in the test
 instead of the dynamic one from the current kernel.

 Tested by successfully running ./test-cap-list and the whole `make check` test
 suite with this patch on a RHEL 7 host.

 v2: Updated the patch to also consider the changes introduced in commit
 4b7c1d5d6a0060 (test-cap-list: always check libcap comes to the same 
 names...)
 ---
  src/shared/cap-list.c| 4 
  src/shared/cap-list.h| 1 +
  src/test/test-cap-list.c | 7 ---
  3 files changed, 9 insertions(+), 3 deletions(-)

 diff --git a/src/shared/cap-list.c b/src/shared/cap-list.c
 index 56d1488..8033e8c 100644
 --- a/src/shared/cap-list.c
 +++ b/src/shared/cap-list.c
 @@ -60,3 +60,7 @@ int capability_from_name(const char *name) {

  return sc-id;
  }
 +
 +int capability_list_length(void) {
 +return (int) ELEMENTSOF(capability_names);
 +}
 diff --git a/src/shared/cap-list.h b/src/shared/cap-list.h
 index c699e46..9824fad 100644
 --- a/src/shared/cap-list.h
 +++ b/src/shared/cap-list.h
 @@ -23,3 +23,4 @@

  const char *capability_to_name(int id);
  int capability_from_name(const char *name);
 +int capability_list_length(void);
 diff --git a/src/test/test-cap-list.c b/src/test/test-cap-list.c
 index 238f876..831cdf5 100644
 --- a/src/test/test-cap-list.c
 +++ b/src/test/test-cap-list.c
 @@ -19,6 +19,7 @@
along with systemd; If not, see http://www.gnu.org/licenses/.
  ***/

 +#include util.h
  #include log.h
  #include cap-list.h
  #include capability.h
 @@ -27,9 +28,9 @@ int main(int argc, char *argv[]) {
  int i;

  assert_se(!capability_to_name(-1));
 -assert_se(!capability_to_name(cap_last_cap()+1));
 +assert_se(!capability_to_name(capability_list_length()));

 -for (i = 0; i = (int) cap_last_cap(); i++) {
 +for (i = 0; i  capability_list_length(); i++) {
  const char *n;

  assert_se(n = capability_to_name(i));
 @@ -45,7 +46,7 @@ int main(int argc, char *argv[]) {
  assert_se(capability_from_name(15) == 15);
  assert_se(capability_from_name(-1) == -EINVAL);

 -for (i = 0; i = (int) cap_last_cap(); i++) {
 +for (i = 0; i  capability_list_length(); i++) {
  _cleanup_cap_free_charp_ char *a = NULL;
  const char *b;
  unsigned u;
 --
 1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/2] test-verbs: add unit tests for verbs minilib

[Filipe Brandenburger]

Hi,

On Sat, Dec 20, 2014 at 8:19 AM, Dave Reisner dreis...@archlinux.org wrote:
 ---
  Makefile.am   |  9 +-
  src/test/test-verbs.c | 78 
 +++
  2 files changed, 86 insertions(+), 1 deletion(-)
  create mode 100644 src/test/test-verbs.c

 diff --git a/Makefile.am b/Makefile.am
 index baa1398..db7dd46 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -1384,7 +1384,8 @@ tests += \
 test-locale-util \
 test-execute \
 test-copy \
 -   test-cap-list
 +   test-cap-list \
 +   test-verbs

Make sure you also add /test-verbs to the top level .gitignore.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [systemd-commits] src/journal

[Filipe Brandenburger]

Hi,

This seems to have fixed test-journal-stream but
test-journal-interleaving is still broken for me, it fails with:

NUMBER=1
NUMBER=2
Assertion 'r == 1' failed at
src/journal/test-journal-interleaving.c:101, function
test_check_numbers_down(). Aborting.
Aborted (core dumped)

Can you please double check?

Can you please make sure that make check passes before pushing the commits?

Thanks!
Filipe


On Fri, Dec 19, 2014 at 6:38 AM, Michal Schmidt
mich...@kemper.freedesktop.org wrote:
  src/journal/sd-journal.c |   66 
 +--
  1 file changed, 30 insertions(+), 36 deletions(-)

 New commits:
 commit 487d37209b30a536636c95479cfeba931fea25c5
 Author: Michal Schmidt mschm...@redhat.com
 Date:   Fri Dec 19 15:05:30 2014 +0100

 journal: fix skipping of duplicate entries in iteration

 I accidentally broke the detection of duplicate entries in 7943f42275
 journal: optimize iteration by returning previously found candidate
 entry.

 When we have a known location of a candidate entry, we must not return
 from next_beyond_location() immediately. We must go through the
 duplicates detection to make sure the candidate differs from the
 already iterated entry.

 This fix slows down iteration a bit, but it's still faster than it
 was before the rework.

 $ time ./journalctl --since=2014-06-01 --until=2014-07-01  /dev/null

 real0m4.448s
 user0m4.298s
 sys 0m0.149s

 (Compare with results from commit 7943f42275, where real was 5.3s before
 the rework.)

 diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c
 index 285e5e3..173f948 100644
 --- a/src/journal/sd-journal.c
 +++ b/src/journal/sd-journal.c
 @@ -412,60 +412,51 @@ _public_ void sd_journal_flush_matches(sd_journal *j) {
  detach_location(j);
  }

 -_pure_ static int compare_with_location(JournalFile *af, Object *ao, 
 Location *l) {
 -uint64_t a;
 -
 -assert(af);
 -assert(ao);
 +_pure_ static int compare_with_location(JournalFile *f, Location *l) {
 +assert(f);
  assert(l);
 +assert(f-location_type == LOCATION_SEEK);
  assert(l-type == LOCATION_DISCRETE || l-type == LOCATION_SEEK);

  if (l-monotonic_set 
 -sd_id128_equal(ao-entry.boot_id, l-boot_id) 
 +sd_id128_equal(f-current_boot_id, l-boot_id) 
  l-realtime_set 
 -le64toh(ao-entry.realtime) == l-realtime 
 +f-current_realtime == l-realtime 
  l-xor_hash_set 
 -le64toh(ao-entry.xor_hash) == l-xor_hash)
 +f-current_xor_hash == l-xor_hash)
  return 0;

  if (l-seqnum_set 
 -sd_id128_equal(af-header-seqnum_id, l-seqnum_id)) {
 +sd_id128_equal(f-header-seqnum_id, l-seqnum_id)) {

 -a = le64toh(ao-entry.seqnum);
 -
 -if (a  l-seqnum)
 +if (f-current_seqnum  l-seqnum)
  return -1;
 -if (a  l-seqnum)
 +if (f-current_seqnum  l-seqnum)
  return 1;
  }

  if (l-monotonic_set 
 -sd_id128_equal(ao-entry.boot_id, l-boot_id)) {
 -
 -a = le64toh(ao-entry.monotonic);
 +sd_id128_equal(f-current_boot_id, l-boot_id)) {

 -if (a  l-monotonic)
 +if (f-current_monotonic  l-monotonic)
  return -1;
 -if (a  l-monotonic)
 +if (f-current_monotonic  l-monotonic)
  return 1;
  }

  if (l-realtime_set) {

 -a = le64toh(ao-entry.realtime);
 -
 -if (a  l-realtime)
 +if (f-current_realtime  l-realtime)
  return -1;
 -if (a  l-realtime)
 +if (f-current_realtime  l-realtime)
  return 1;
  }

  if (l-xor_hash_set) {
 -a = le64toh(ao-entry.xor_hash);

 -if (a  l-xor_hash)
 +if (f-current_xor_hash  l-xor_hash)
  return -1;
 -if (a  l-xor_hash)
 +if (f-current_xor_hash  l-xor_hash)
  return 1;
  }

 @@ -740,21 +731,24 @@ static int next_beyond_location(sd_journal *j, 
 JournalFile *f, direction_t direc
  f-last_n_entries = n_entries;

  if (f-last_direction == direction  f-current_offset  0) {
 +cp = f-current_offset;
 +
  /* LOCATION_SEEK here means we did the work in a previous
   * iteration and the current location already points to a
   * candidate entry. */
 -if (f-location_type == LOCATION_SEEK)
 -return 1;
 -
 -cp = f-current_offset;
 +if (f-location_type != 

Re: [systemd-devel] [PATCH] build: add option to disable hwdb

[Filipe Brandenburger]

On Fri, Dec 19, 2014 at 6:32 AM, Michael Biebl mbi...@gmail.com wrote:
 This also removed the HAVE_PYTHON section from Makefile-man.am. Why it
 did that is unclear to me.

This happened to me before and when it did it was because ./configure
didn't detect a working Python, so make update-man-list didn't use
one.

Note that for it to find a working Python you need both a python
interpreter *and* the python-lxml module installed.

HTH,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] build: add option to disable hwdb

[Filipe Brandenburger]

On Fri, Dec 19, 2014 at 8:31 AM, Michael Biebl mbi...@gmail.com wrote:
 2014-12-19 17:10 GMT+01:00 Filipe Brandenburger filbran...@google.com:
 On Fri, Dec 19, 2014 at 6:32 AM, Michael Biebl mbi...@gmail.com wrote:
 This also removed the HAVE_PYTHON section from Makefile-man.am. Why it
 did that is unclear to me.

 This happened to me before and when it did it was because ./configure
 didn't detect a working Python, so make update-man-list didn't use
 one.

 Hm, I have both installed and the ./configure summary has

 Python:  yes
 Python Headers:  yes
 man pages:   yes

 Still, the result is the same. I'm puzzled

Ok I could reproduce it if man/systemd.index.xml and
man/systemd.directives.xml were not present when update-man-list
runs...

Try running make man/systemd.index.xml first (it should generate
both of them) and then make update-man-list that should work.

Maybe that should be made into a dependency of update-man-list?

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [systemd-commits] src/journal

[Filipe Brandenburger]

On Fri, Dec 19, 2014 at 8:30 AM, Michal Schmidt mschm...@redhat.com wrote:
 On 12/19/2014 04:49 PM, Filipe Brandenburger wrote:
 This seems to have fixed test-journal-stream but
 test-journal-interleaving is still broken for me, it fails with:

 NUMBER=1
 NUMBER=2
 Assertion 'r == 1' failed at
 src/journal/test-journal-interleaving.c:101, function
 test_check_numbers_down(). Aborting.
 Aborted (core dumped)

 Can you please double check?

 Hello Filipe,

 thanks for pinging me about this.

 This should be fixed now with 668c965af4e journal: skipping of
 exhausted journal files is bad if direction changed.

It's working now, thanks!
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Journal tests broken by commit 6573ef05a3cbe1 (journal: keep per-JournalFile location info during iteration)

[Filipe Brandenburger]

Hi,

Commit 6573ef05a3cbe1 (journal: keep per-JournalFile location info
during iteration) breaks tests test-journal-stream and
test-journal-interleaving.

It seems that the logic of overriding f-current_offset in
journal_file_save_location has other unintended side effects, checking
out that commit and commenting out that line seems to have
test-journal-stream working back again, but not
test-journal-interleaving. The same no longer has effect in trunk
head anymore since I believe the follow up patches rely on that saved
information which is no longer there.

Not sure if the problem is in the code or in the tests, but looking at
the tests the logic looks right to me, so I'm leaning towards the
code...

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Journal tests broken by commit 6573ef05a3cbe1 (journal: keep per-JournalFile location info during iteration)

[Filipe Brandenburger]

I took a closer look at test-journal-stream. It's setting up 3
journal files and writing entries to the three of them with some
interleaving, then expecting to read them in order.

After commit 6573ef05a3cbe1, what happens is that a single call to
sd_journal_next() ends up calling next_beyond_location() on the three
files which advances the f-current_offset from the three of them, so
the first access looks right, you'll be comparing the first entry of
each of the three files, but the second access is bad because you're
comparing the second entry of each of the three files, instead of the
second entry of the one that was picked before with the first entry of
the other two files.

Not sure what's the correct solution, maybe journal_file_save_location
needs to happen only in real_journal_next() outside the
ORDERED_HASHMAP_FOREACH loop? I'll try that and report if I find
something that seems to solve this problem...

Cheers,
Filipe


On Thu, Dec 18, 2014 at 2:34 PM, Filipe Brandenburger
filbran...@google.com wrote:
 Hi,

 Commit 6573ef05a3cbe1 (journal: keep per-JournalFile location info
 during iteration) breaks tests test-journal-stream and
 test-journal-interleaving.

 It seems that the logic of overriding f-current_offset in
 journal_file_save_location has other unintended side effects, checking
 out that commit and commenting out that line seems to have
 test-journal-stream working back again, but not
 test-journal-interleaving. The same no longer has effect in trunk
 head anymore since I believe the follow up patches rely on that saved
 information which is no longer there.

 Not sure if the problem is in the code or in the tests, but looking at
 the tests the logic looks right to me, so I'm leaning towards the
 code...

 Cheers,
 Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Journal tests broken by commit 6573ef05a3cbe1 (journal: keep per-JournalFile location info during iteration)

[Filipe Brandenburger]

On Thu, Dec 18, 2014 at 4:39 PM, Filipe Brandenburger
filbran...@google.com wrote:
 Not sure what's the correct solution, maybe journal_file_save_location
 needs to happen only in real_journal_next() outside the
 ORDERED_HASHMAP_FOREACH loop? I'll try that and report if I find
 something that seems to solve this problem...

I tried this on top of commit 6573ef05a3cbe1 and it solves the problem.

After applying this patch:

diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c
index 71b056c..a727e12 100644
--- a/src/journal/sd-journal.c
+++ b/src/journal/sd-journal.c
@@ -859,8 +859,6 @@
 found = true;

 if (found) {
-journal_file_save_location(f, direction, c, cp);
-
 if (ret)
 *ret = c;
 if (offset)
@@ -917,6 +915,8 @@
 if (!new_file)
 return 0;

+journal_file_save_location(new_file, direction, o, new_offset);
+
 r = journal_file_move_to_object(new_file, OBJECT_ENTRY,
new_offset, o);
 if (r  0)
 return r;


Then test-journal-stream and test-journal-interleaving are working
again and I have a clean make check run.

But this does not work on trunk head, even after adapting it, the
tests start to fail in a different location, probably because of the
changes that come after it, so I think we'll need this and further
changes to it. I'll keep looking.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Journal tests broken by commit 6573ef05a3cbe1 (journal: keep per-JournalFile location info during iteration)

[Filipe Brandenburger]

On Thu, Dec 18, 2014 at 4:58 PM, Filipe Brandenburger
filbran...@google.com wrote:
 But this does not work on trunk head, even after adapting it, the
 tests start to fail in a different location, probably because of the
 changes that come after it, so I think we'll need this and further
 changes to it. I'll keep looking.

Ok, so I found that reverting these three patches:

- Revert 58439db journal: drop unnecessary parameters of
next_beyond_location()
- Revert d8ae66d journal: compare candidate entries using
JournalFiles' locations
- Revert e499c99 journal: remove redundant variable new_offset

And applying the following patch:

diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c
index 8d63094..cabe080 100644
--- a/src/journal/sd-journal.c
+++ b/src/journal/sd-journal.c
@@ -860,8 +860,6 @@
 found = true;

 if (found) {
-journal_file_save_location(f, direction, c, cp);
-
 if (ret)
 *ret = c;
 if (offset)
@@ -918,6 +916,8 @@
 if (!new_file)
 return 0;

+journal_file_save_location(new_file, direction, o, new_offset);
+
 r = journal_file_move_to_object(new_file, OBJECT_ENTRY,
new_offset, o);
 if (r  0)
 return r;


This seems to fix test-journal-stream, but
test-journal-interleaving is still broken (assertion 'r == 1' failed
at src/journal/test-journal-interleaving.c:101, function
test_check_numbers_down.)

At this point, I'm leaning towards believeing that the logic of the
patchset doesn't really work with more than one journal file and I'd
be inclined to suggest reverting all of it.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] serialization bug, swap bug, etc.

[Filipe Brandenburger]

On Wed, Dec 10, 2014 at 4:11 AM, Lennart Poettering
lenn...@poettering.net wrote:
 In fact, I think we should drop the
 libcap dependency altogether and just do the two syscalls it offers to
 us natively in systemd code. Neither is libcap a particularly nice
 library, nor is the stuff it does particularly complex, hence we can
 as well wrap the two calls we need in our code.

I started looking at this and I just sent a patch set to remove the
include of sys/capability.h where it was not really in use.

Regarding the valid uses of libcap, it looks like the non-trivial part
is cap_to_text/cap_from_text which we would have to reimplement and
possibly keep them in sync with libcap.

libcap also tries to support kernels which only support 32-bit
capabilities. If we replace that code, should we make an assumption of
64-bit capabilities and just use a uint64_t to represent the bits?

Let me know if you think this is something worth doing and I can
contribute an implementation.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/3] test-compress: make sure asserts with side effects use assert_se()

[Filipe Brandenburger]

Otherwise the test fails when built with CPPFLAGS='-DNDEBUG' which disables
assertions.

Tested:
- make check TESTS='test-compress' CPPFLAGS='-DNDEBUG'
---
 src/journal/test-compress.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/journal/test-compress.c b/src/journal/test-compress.c
index f5f5f8df3902..026d630ac2a8 100644
--- a/src/journal/test-compress.c
+++ b/src/journal/test-compress.c
@@ -145,11 +145,11 @@ static void test_compress_stream(int compression,
 
 assert_se((dst = mkostemp_safe(pattern, O_RDWR|O_CLOEXEC)) = 0);
 
-assert(compress(src, dst, -1) == 0);
+assert_se(compress(src, dst, -1) == 0);
 
 if (cat) {
 assert_se(asprintf(cmd, %s %s | diff %s -, cat, pattern, 
srcfile)  0);
-assert(system(cmd) == 0);
+assert_se(system(cmd) == 0);
 }
 
 log_debug(/* test decompression */);
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/3] test-path-util: use assert_se in all assertions

[Filipe Brandenburger]

Otherwise they get optimized out when CPPFLAGS='-DNDEBUG' is used, and that
causes the tests to fail.

Tested:
- make check TESTS='test-path-util' CPPFLAGS='-DNDEBUG'
---
 src/test/test-path-util.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index c8dcd853978f..01afb3e6feed 100644
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -79,35 +79,35 @@ static void test_path(void) {
 char p2[] = //aaa/.ccc;
 char p3[] = /./;
 
-assert(path_equal(path_kill_slashes(p1), aaa/bbb/ccc));
-assert(path_equal(path_kill_slashes(p2), /aaa/./ccc));
-assert(path_equal(path_kill_slashes(p3), /./));
+assert_se(path_equal(path_kill_slashes(p1), aaa/bbb/ccc));
+assert_se(path_equal(path_kill_slashes(p2), /aaa/./ccc));
+assert_se(path_equal(path_kill_slashes(p3), /./));
 }
 }
 
 static void test_find_binary(const char *self) {
 char *p;
 
-assert(find_binary(/bin/sh, p) == 0);
+assert_se(find_binary(/bin/sh, p) == 0);
 puts(p);
-assert(streq(p, /bin/sh));
+assert_se(streq(p, /bin/sh));
 free(p);
 
-assert(find_binary(self, p) == 0);
+assert_se(find_binary(self, p) == 0);
 puts(p);
-assert(endswith(p, /test-path-util));
-assert(path_is_absolute(p));
+assert_se(endswith(p, /test-path-util));
+assert_se(path_is_absolute(p));
 free(p);
 
-assert(find_binary(sh, p) == 0);
+assert_se(find_binary(sh, p) == 0);
 puts(p);
-assert(endswith(p, /sh));
-assert(path_is_absolute(p));
+assert_se(endswith(p, /sh));
+assert_se(path_is_absolute(p));
 free(p);
 
-assert(find_binary(-, p) == -ENOENT);
+assert_se(find_binary(-, p) == -ENOENT);
 
-assert(find_binary(/some/dir/-, p) == -ENOENT);
+assert_se(find_binary(/some/dir/-, p) == -ENOENT);
 }
 
 static void test_prefixes(void) {
@@ -156,8 +156,8 @@ static void test_prefixes(void) {
 
 b = false;
 PATH_FOREACH_PREFIX_MORE(s, ) {
-assert(!b);
-assert(streq(s, ));
+assert_se(!b);
+assert_se(streq(s, ));
 b = true;
 }
 }
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 3/3] test-util: use assert_se() for call to safe_mkdir with side effect

[Filipe Brandenburger]

Otherwise it gets optimized out when CPPFLAGS='-DNDEBUG' is used.

Tested:
- make check TESTS='test-util' CPPFLAGS='-DNDEBUG'
---
 src/test/test-util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/test/test-util.c b/src/test/test-util.c
index 34d5f2ed7d23..4d9b28f9c8e7 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -863,7 +863,7 @@ static void test_readlink_and_make_absolute(void) {
 char name_alias[] = /tmp/test-readlink_and_make_absolute-alias;
 char *r = NULL;
 
-assert(mkdir_safe(tempdir, 0755, getuid(), getgid()) = 0);
+assert_se(mkdir_safe(tempdir, 0755, getuid(), getgid()) = 0);
 assert_se(touch(name) = 0);
 
 assert_se(symlink(name, name_alias) = 0);
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] Revert build-sys: include PolicyKit files as part of distribution

[Filipe Brandenburger]

+Zbigniew who committed the original patch.

On Fri, Jul 4, 2014 at 11:43 AM, Mike Gilbert flop...@gentoo.org wrote:
 This reverts commit 0c26bfc3d21fdb3963f1248c237e2f1a33b5566d.

 src/core/org.freedesktop.systemd1.policy.in.in depends on values which
 are specified at configure time, so we cannot ship the corresponding
 policy file in the tarball.

 Since we need to regenerate one policy file, we might as well generate
 them all.

I'm OK with this revert. With this patch I can still build it without
intltool present by passing ./configure --disable-polkit which will
not try to build them so that's fine.

Sorry I didn't notice the dependency of
org.freedesktop.systemd1.policy on @rootlibexecdir@ and @bindir@
variables when I first suggested the patch.

I briefly looked into whether it's possible to first do
internationalization of the template and generate an internationalized
version with the variables still present and then do the variable
substitution as a second step, but that requires either changing the
name of the files (because the intltool rules assume they'll generate
the *.policy file) or overriding/ignoring the intltool m4 macros. I
might still send a patch for that but let's not hold this revert on
that.

Cheers,
Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] networkd: fix alignment of gperf source

[Filipe Brandenburger]

---
 src/network/networkd-network-gperf.gperf | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/network/networkd-network-gperf.gperf 
b/src/network/networkd-network-gperf.gperf
index 3aaae4c94bff..ce9047cd06c4 100644
--- a/src/network/networkd-network-gperf.gperf
+++ b/src/network/networkd-network-gperf.gperf
@@ -47,8 +47,8 @@ DHCP.UseDNS, config_parse_bool,   
   0,
 DHCP.UseMTU, config_parse_bool,  0,
 offsetof(Network, dhcp_mtu)
 DHCP.UseHostname,config_parse_bool,  0,
 offsetof(Network, dhcp_hostname)
 DHCP.UseDomainName,  config_parse_bool,  0,
 offsetof(Network, dhcp_domainname)
-DHCP.UseRoutes,config_parse_bool,  0,  
   offsetof(Network, dhcp_routes)
-DHCP.SendHostname, config_parse_bool,  0,  
   offsetof(Network, dhcp_sendhost)
+DHCP.UseRoutes,  config_parse_bool,  0,
 offsetof(Network, dhcp_routes)
+DHCP.SendHostname,   config_parse_bool,  0,
 offsetof(Network, dhcp_sendhost)
 DHCP.CriticalConnection, config_parse_bool,  0,
 offsetof(Network, dhcp_critical)
 /* backwards compatibility: do not add new entries to this section */
 DHCPv4.UseDNS,   config_parse_bool,  0,
 offsetof(Network, dhcp_dns)
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [systemd-commits] 5 commits - Makefile-man.am man/daemon.xml man/file-hierarchy.xml tmpfiles.d/systemd.conf

[Filipe Brandenburger]

On Mon, Jun 30, 2014 at 2:38 PM, Lennart Poettering
lenn...@poettering.net wrote:
 Maybe it should try both and figure out which one of them exists?

 Or default to $libdir and allow an override for the cross-distro cases.

 Hmm, I am tempted to say that we should add a logic to this that just
 checks the few debian-style arch tuples we know of, plus fedora-style
 lib64, and if any of them exist, we create a lib64 symlink to them. The
 table should be relatively short I think. Ugly that we need this, but
 maintainable.

 With that in place we should allow cross-distro nspawn I figure.

Sounds good to me.

Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] [RFC] Add binary password agent protocol

[Filipe Brandenburger]

On Thu, Jun 26, 2014 at 4:54 PM, David Härdeman da...@hardeman.nu wrote:
 Add binary string handling functions and extend the password agent
 protocol to support binary strings (using = as a string prefix
 instead of +).

Please also add /test-bstrv to .gitignore.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 0/3] build-sys: do not require intltool when building from archive

[Filipe Brandenburger]

Hi Zbigniew,

This set of patches does explicit handling of --disable-nls and some extra
detection of intltool in order to make configure not fail when intltool is not
present.

It also ships the translated *.policy files in the distribution tarball.

This makes it possible to build from an archive without having intltool
installed, as the systemd README implies (intltool is only listed under When
building from git, you need the following additional dependencies.)

Please take a look and let me know what you think of these.

Cheers,
Filipe


Filipe Brandenburger (3):
  build-sys: add explicit support for --diable-nls
  build-sys: check if intltool is present before initializing it
  build-sys: include PolicyKit files as part of distribution

 Makefile.am  |  2 +-
 configure.ac | 14 ++
 2 files changed, 15 insertions(+), 1 deletion(-)

-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/3] build-sys: disable NLS support if intltool is not found

[Filipe Brandenburger]

IT_PROG_INTLTOOL makes configure fail if intltool is not present.  If we can
not find intltool, then disable NLS (otherwise make in po/ fails since MSGFMT
will not be defined.)

Tested: Built it on a host without intltool.
  $ ./configure --enable-nls
  ...
  checking for intltool-merge... no
  configure: error: --enable-nls requested but intltool not found

  $ ./configure --disable-polkit
  ...
  checking for intltool-merge... no
  configure: WARNING: *** Disabling NLS support because intltool was not found
  checking whether NLS is requested... no
  ...
  $ make
---
 configure.ac | 12 
 1 file changed, 12 insertions(+)

diff --git a/configure.ac b/configure.ac
index 31879d3fb410..93aba067397c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -49,6 +49,18 @@ AS_IF([test x$enable_static = xyes], 
[AC_MSG_ERROR([--enable-static is not s
 AS_IF([test x$enable_largefile = xno], [AC_MSG_ERROR([--disable-largefile 
is not supported by systemd])])
 
 # i18n stuff for the PolicyKit policy files
+
+# Check whether intltool can be found, disable NLS otherwise
+AC_CHECK_PROG(intltool_found, [intltool-merge], [yes], [no])
+AS_IF([test x$intltool_found != xyes],
+  [AS_IF([test x$enable_nls = xyes],
+ [AC_MSG_ERROR([--enable-nls requested but intltool not found])],
+ [AS_IF([test x$enable_nls != xno],
+[AC_MSG_WARN([*** Disabling NLS support because intltool 
was not found])
+ enable_nls=no])
+ ])
+  ])
+
 AM_NLS
 AS_IF([test x$enable_nls != xno], [
 # intltoolize greps for '^(AC|IT)_PROG_INTLTOOL', so it needs to be on its 
own line
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 3/3] build-sys: include PolicyKit files as part of distribution

[Filipe Brandenburger]

So that building from an archive works even if intltool is not present.
The README file already mentioned that intltool should only be required
when building from git.

Tested: Built it from the distribution archive on a host without intltool.
  $ ./configure --enable-polkit
  $ make
---
 Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 4b292b210531..94ea4d10ae0c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5170,7 +5170,7 @@ units/user/%: units/%.m4
$(AM_V_M4)$(M4) -P $(M4_DEFINES) -DFOR_USER=1  $  $@
 
 if ENABLE_POLKIT
-nodist_polkitpolicy_DATA = \
+dist_polkitpolicy_DATA = \
$(polkitpolicy_files) \
$(polkitpolicy_in_in_files:.policy.in.in=.policy)
 endif
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/3] build-sys: add explicit support for --diable-nls

[Filipe Brandenburger]

In particular, disable intltool when --disable-nls is passed to configure.

Tested: Built it on a host without intltool or gettext.
  $ ./configure --disable-nls --disable-polkit
  $ make
---
 configure.ac | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/configure.ac b/configure.ac
index bb6018f87dec..31879d3fb410 100644
--- a/configure.ac
+++ b/configure.ac
@@ -49,7 +49,17 @@ AS_IF([test x$enable_static = xyes], 
[AC_MSG_ERROR([--enable-static is not s
 AS_IF([test x$enable_largefile = xno], [AC_MSG_ERROR([--disable-largefile 
is not supported by systemd])])
 
 # i18n stuff for the PolicyKit policy files
+AM_NLS
+AS_IF([test x$enable_nls != xno], [
+# intltoolize greps for '^(AC|IT)_PROG_INTLTOOL', so it needs to be on its 
own line
 IT_PROG_INTLTOOL([0.40.0])
+])
+
+AS_IF([test -z $INTLTOOL_POLICY_RULE], [
+# If intltool is not available, provide a dummy rule to fail generation of 
%.policy files with a meaningful error message
+INTLTOOL_POLICY_RULE='%.policy: %.policy.in ; @echo   ITMRG$@  
echo *** intltool support required to build target $@  false'
+AC_SUBST(INTLTOOL_POLICY_RULE)
+])
 
 GETTEXT_PACKAGE=systemd
 AC_SUBST(GETTEXT_PACKAGE)
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/3] sd-dhcp6-client: remove bogus dependency on dhcp6-icmp6.h

[Filipe Brandenburger]

Makefile.am had a reference to it but it none of the sources included it.

Tested: make dist works again after this patchset is applied.

Fixes: 2ea8857effb833615b16d10fc7a19a7104c19e13
---
 Makefile.am | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 4f4eb3729353..1b861873da51 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2680,7 +2680,6 @@ test_icmp6_rs_LDADD = \
 
 test_dhcp6_client_SOURCES = \
src/systemd/sd-dhcp6-client.h \
-   src/libsystemd-network/dhcp6-icmp6.h \
src/libsystemd-network/dhcp6-internal.h \
src/libsystemd-network/test-dhcp6-client.c
 
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel