Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing edges to systemd-analyze dot #11555

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

roberthoffmann
Copy link

Add BindsTo, OnFailure, Triggers. Make colors and styles
configurable. Add analyze.conf manpage, /etc/systemd/analyze.conf

@roberthoffmann roberthoffmann changed the title Add missing edges to systemd-analyze Add missing edges to systemd-analyze dot Jan 25, 2019
Copy link
Member

@keszybz keszybz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making this configurable is a bit over-the-top, but the patch looks OK.
Adding analyze.conf just for this would be strange, but I expect we'll want to add more configuration (in particular for systemd-analyze security) so it makes sense to have a configuration file. But I think we should allow the user to create their own configuration, i.e. we should look at paths under ~/.config/systemd/.

man/analyze.conf.xml Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
@keszybz keszybz added analyze reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Jan 26, 2019
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
man/analyze.conf.xml Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
@poettering
Copy link
Member

we usually want to merge "perfect" PRs, i.e. PRs where each commit makes logical sense, but not necessarily historical one. i.e. PRs that are perfectly bisectable, where on each commit all test cases pass and that makes sense on its own.

Consider using this:

git rebase origin/master -x 'ninja -C build test'

to make sure that your PR remains in fully bisectable state.

src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
poettering added a commit to poettering/systemd that referenced this pull request Mar 11, 2019
evverx added a commit to evverx/systemd that referenced this pull request Mar 11, 2019
…l query

to make it easier to comlain about `strtok` :-)

Inspired by systemd#11963, which, in turn,
was prompted by systemd#11555.
poettering pushed a commit that referenced this pull request Mar 12, 2019
…l query

to make it easier to comlain about `strtok` :-)

Inspired by #11963, which, in turn,
was prompted by #11555.
jkartseva pushed a commit to jkartseva/systemd that referenced this pull request Mar 18, 2019
…l query

to make it easier to comlain about `strtok` :-)

Inspired by systemd#11963, which, in turn,
was prompted by systemd#11555.
@roberthoffmann roberthoffmann force-pushed the systemd-analyze_add_missing_edges branch 2 times, most recently from e34e073 to 2fdfe97 Compare March 29, 2019 16:24
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
return log_oom();
if (r < 0) {
log_error("Invalid syntax, ignoring: %s", arg_dot_color_style);
return r;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return log_error_errno(r, …

Also, you are not ignoring the error here, don#t say you do

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return and log_error lines still need to be folded into one

src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
src/analyze/analyze.c Outdated Show resolved Hide resolved
roberthoffmann and others added 6 commits April 12, 2019 16:56
Add BindsTo, OnFailure, Triggers. Make colors and styles
configurable. Add analyze.conf manpage, /etc/systemd/analyze.conf
Add user configuration, use arrays, add PartOf.
Add options --failure and --triggers to shell completion.
Add option --dependency-color-style
Options dependency-kind, dependency-color, dependency-style, helper
function, parsing, etc.
@roberthoffmann roberthoffmann force-pushed the systemd-analyze_add_missing_edges branch from 2fdfe97 to 5c18fd5 Compare April 12, 2019 15:30
const char *style,
char* patterns[],
char* from_patterns[],
char* to_patterns[]) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base automatically changed from master to main January 21, 2021 11:54
mrc0mmand pushed a commit to mrc0mmand/rhel-8 that referenced this pull request Dec 6, 2021
…l query

to make it easier to comlain about `strtok` :-)

Inspired by systemd/systemd#11963, which, in turn,
was prompted by systemd/systemd#11555.

(cherry picked from commit 7ba5ded)

Related: #2017033
systemd-rhel-bot pushed a commit to redhat-plumbers/systemd-rhel8 that referenced this pull request Dec 6, 2021
…l query

to make it easier to comlain about `strtok` :-)

Inspired by systemd/systemd#11963, which, in turn,
was prompted by systemd/systemd#11555.

(cherry picked from commit 7ba5ded)

Related: #2017033
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyze needs-rebase reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks
Development

Successfully merging this pull request may close these issues.

None yet

4 participants