It really must not store any packaged files in user home directory, and that it includes /root.


> %clean
> make distclean

This section is for more general cleaning up after a build and should not be overridden like that.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_sections


> %defattr(-,root,root,-)

https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_permissions


> duc.x86_64: W: incoherent-version-in-changelog 1.4.4-9.git5a40efd ['1.4.4-8.git5a40efd.fc30', '1.4.4-8.git5a40efd']

Your package release is 8, while your changelog entry refers to 9.

Michael, thanks for your review - I fixed the things you mentioned except this one:

> It really must not store any packaged files in user home directory, and that it includes /root.

I'm unsure about how to make duc behave in the way I want then - see the remark about proposed usage.

The possibilities I came up with are these:

- Provide a system-wide ducrc: Doesn't work, as this would need user-specific default overrides.
- Provide a rc file for root: Against packaging policy.
- Set DUC_DATABASE=/var/cache/duc for root (and only root):
  - /etc/environment is out, as it concerns all users.
  - Dotfiles for root are out, packaging policy again.
  - /etc/profile.d/ drop-in works, but is shell-specific.

So should I drop this and consider this not the packager's job, or is there something I'm missing?

/root/.ducrc is not possible and very wrong. It should be quite simple to just make that directory default, if uid=0 was detected. Runtime from the code. Just make some ifs in the code.

Or pass db directory to service parameters. System level service would use system cache, user started service might use just home cache.

It might also just check access rights to default system cache. If no right to write there, just create and use cache directory in $HOME/.cache/duc.
Please take into account it might run under sudo, where $HOME is not root's. It might work to provide also shell wrapper, which would change DUC_DATABASE.

For example:
#!/bin/sh

if [ "$(id -u)" = 0 ]; then
  DUC_DATABASE=/var/cache/duc
fi

duc "$@"

But I think getuid() kernel call in the code would be better than similar shell wrapper. Check man 2 getuid. But I like access("/var/cache/duc", W_OK) check more, because it would allow also access by group rights.

It is quite unusual to provide own selinux policy files. Why are they required?

Also, it seems wrong to have privileged duc index command and duc gui for displaying it. I think duc-index separate command would be more secure. Only it would have elevated privileges to read other users data.
Reading indexed data should not require any extra privileges, just read permissions to indexed database. But current binary cannot set different selinux contexts to different actions. Maybe duc-indexd would be better name, to indicate it is usually run as a daemon, indexing service. It might work with shell wrapper around duc index, which would have different context. Similar to what I used above.

It seems like this is advanced ncdu tool with a bundled cache. Might be quite nice to have.

(In reply to Robert Führicht from comment #2)
> Michael, thanks for your review - I fixed the things you mentioned except
> this one:
> 
> > It really must not store any packaged files in user home directory, and that it includes /root.
> 
> I'm unsure about how to make duc behave in the way I want then - see the
> remark about proposed usage.
> 
> The possibilities I came up with are these:
> 
> - Provide a system-wide ducrc: Doesn't work, as this would need
> user-specific default overrides.
> - Provide a rc file for root: Against packaging policy.
> - Set DUC_DATABASE=/var/cache/duc for root (and only root):
>   - /etc/environment is out, as it concerns all users.
>   - Dotfiles for root are out, packaging policy again.
>   - /etc/profile.d/ drop-in works, but is shell-specific.
> 
> So should I drop this and consider this not the packager's job, or is there
> something I'm missing?

Why don't you set environmnent (In reply to Robert Führicht from comment #2)
> Michael, thanks for your review - I fixed the things you mentioned except
> this one:
> 
> > It really must not store any packaged files in user home directory, and that it includes /root.
> 
> I'm unsure about how to make duc behave in the way I want then - see the
> remark about proposed usage.
> 
> The possibilities I came up with are these:
> 
> - Provide a system-wide ducrc: Doesn't work, as this would need
> user-specific default overrides.
> - Provide a rc file for root: Against packaging policy.
> - Set DUC_DATABASE=/var/cache/duc for root (and only root):
>   - /etc/environment is out, as it concerns all users.
>   - Dotfiles for root are out, packaging policy again.
>   - /etc/profile.d/ drop-in works, but is shell-specific.
> 
> So should I drop this and consider this not the packager's job, or is there
> something I'm missing?

Maybe set DUC_DATABASE from systemd unit. I doubt there is reason why root should have different behaviour from other users. Instead, prepare systemd service for running the service on system level. And allow users to run own instances as user level. I have suggested few ways. Is any of them suitable to you?

Due to lack of responses I leave this review for anyone other.

This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.