Closed bluca closed 5 months ago
Hello @bluca Thank you very much for this well detailed information. I was able to build the project on my system.
I do not seem quite to understand well what you meant by adding Hello World from <name>!
log message to a tool under src/. There are multiple directories inside src which mostly contain C code and header files.
Output Input
This is for the repeatable task one.
Looks good, you may proceed with the next short repeatable task
The next short repeatable task which is mainly the combination of the last two 1. 2.
Referenced code portion
The next short repeatable task which is mainly the combination of the last two
Looks good, you may proceed with the next step, short tasks
This is my submission for the repeatable task 1:
change:
output:
hello @bluca I am interested in this issue #29609. I have been able to see the file to be changed (70-mouse.hwdb). I also tried to find if there is any test for the hwdb.d but, there is none. Also, kindly give any suggestion regarding that.
This is my submission for the repeatable task 1:
Looks good, you may proceed to the next step
hello @bluca I am interested in this issue #29609. I have been able to see the file to be changed (70-mouse.hwdb). I also tried to find if there is any test for the hwdb.d but, there is none. Also, kindly give any suggestion regarding that.
hwdb is just a list of IDs, and requires the exact involved hardware to test, so I'd recommend to avoid it as a task
Thank you for that.
@bluca I saw this issue #23703. which about formating, I want to improve the formatting by adding AlignArrayOfStructures
which as suggested. I went through the clang format documentation and I saw this.
I am planning to use the left options for that.
Should I proceed ?
Sure, go for it. Our current style is "right justify", so please use that option.
@bluca should I proceed to take another task from "fix one of the issues labelled as "please-submit-as-pr" or proceed to "improving the existing unit test" ?
You can pick from any of the "short tasks", there's no order requirement. Please consider choosing something that involves code changes, as that makes it easier to move to the next stage
This is my submission for repeatable task 2:
The systemd tool ‘ac-power’ detects whether the machine is connected to an external power source or not. It can be used as such:
The code for the tool consists of a single file ‘ac-power.c’ and can be found in systemd/src/ac-power. It consists of three functions: help(), parse_argv() and run().
The help() function is the simplest one and prints a string to stdout upon invocation. The string mentions the role of the tool and the possible arguments that can be passed.
This function parses the arguments passed to the tool through the help of GNU’s getopt.h
. The start of the function defines an array of option structs which represents the valid paramters that can be passed to the tool. The struct consists of four fields:
This array of structs can be then passed to the getopt_long()
function which is responsible for actually decoding the options. The next few lines of assert() check for valid input and a variable (int c) is defined to store the return value of getopt_long()
.
Next, a while loop is run until the value of c is ‘-1’, which represents no more options being left. On each iteration of the loop, the updated value of ‘c’ is passed to a switch statement which determines the action needed to be performed.
The case c = ‘h’ calls the help() function which results in the help message being printed to stdout.
The case c = ‘v’ turns the value of the flag / global variable ‘arg_verbose’ to true, which is then parsed by run() to print ‘yes’ or ‘no’ to stdout, depending upon the options being passed.
The case c = ARG_VERSION means the option ‘--version’ was passed and hence, version() is called which prints the package version to stdout.
The case c = ARG_LOW represents the option ‘–-low’ being passed which simply changes the value of the enum variable arg_action to ACTION_LOW. The futher action that is required is performed inside of run().
The value c = ‘?’ represents an invalid option being passed. If this case in encountered, getopt_long()
has already printed an error message so the -EINVAL error code is simply returned.
The default case should never be reached so in the case that it is reached, the program is terminated with assert_not_reached()
.
This function is defined as the entry point for the tool/program with the macro DEFINE_MAIN_FUNCTION_WITH_POSITIVE_FAILURE()
. The function starts by defining a variable (int r) to store the value to be returned. The next two functions calls, log_parse_environment()
and log_open()
prepare for logging, decoding environment variables and other such general functionality. Then, the parse_argv()
function is called and its return value is stored in ‘r’. A value of 0 indicates the ‘—help’ or ‘--version’ option was passed and hence, the required message has already been printed and a negative value indicates an error. The program is terminated in both the cases.
Next, the arg_action flag is queried. Note that this flag has been assigned the required value through parse_argv()
. If arg_action has value ‘ACTION_AC_POWER’, ‘r’ is updated with the return value of on_ac_power()
, which is ‘1’ if ac power is connected, ‘0’ if not and ‘-1’ otherwise. An error is logged in case of r = -1.
If the value of arg_action has been assigned to ‘ACTION_LOW’ instead, the function battery_is_discharging_or_low()
is called which, similar to the above case, returns ‘1’ if the battery is discharging or low, ‘0’ if not and ‘-1’ otherwise. An error is logged in case of r = -1.
If the ‘arg_verbose’ flag has been set to true, ‘yes’ or ‘no’ is printed to stdout, depending upon the parameters being passed. At the end, the value opposite to that of ‘r’ is returned (since the function must return with positive failure), which is meant to be used by scripts and other functions.
This is my submission for repeatable task 2:
Looks good, you may proceed with the next step
My Submission for repeatable Task 1 Input Code: Output:
Looks good, you may proceed with the next step
Right, I have a question regarding the next step. Do we absolutely need to work on an idea from the short tasks list or is it fine if we decide to work on a topic mentioned in the long tasks list as the next step?
Looks good, you may proceed with the next step
Right, I have a question regarding the next step. Do we absolutely need to work on an idea from the short tasks list or is it fine if we decide to work on a topic mentioned in the long tasks list as the next step?
Please start with a short task first
My Submission for repeatable Task 1
Looks good, please go ahead
My submission for repeatable Task2
The systemd-ask-password utility provides a secure way for receiving a password
The code which is located in the src/ask-password/ask-password.c file contains included standard C libraries for error handling, and other system-level operations. It also contains three functions which include help(), parse_argv() and run() for successfully implementing this utility.
The help function defines the message displayed when the user runs systemd-ask-password command with the -h or --help option. The help function return 0 to indicate successful operation
This function is responsible for parsing the command line arguments.
It defines constants for each command line long option using enumerations. it then defines an array of struct options representing the long options accepted by the program.
It then declares variables c and r for options parsing and resets optind
to 0.
it sets and processes command line options and sets the value to c using getopt_long
.
using the values gotten from c from getopt_long
, the switch statement handles each and sets corresponding variables
An if statement handles the --emoji options. it determines if to show an emoji based on the user's inputs.
it then sets the variable arg_message based on the remaining command line options or defaults to "input" if using echo. The function returns 1 on successful operation
The run function defines the main function for executing the utility. it the initializes logging, parses command lines arguements using the parse_argv function and sets its value to r and it also checks for errors. it sets a timeout for the password request and initializes a structure AskPasswordRequest with the information gotten from the command line arguments It then invokes the ask_password_auto function to query the user for a password and sets the value to r . It then prints the obtained password to stdout based on the options provided.he function returns 0 to indicate success. lastly the module defines the main function using the DEFINE_MAIN_FUNCTION macro.
My submission for repeatable Task2
Looks good, please go ahead with the next step
@bluca I added more unit test to cover uid_range_covers function in uid-range.c. But, after trying to test the whole project, I am getting this 1 failed test. Kindly help to look at it. Thank you.
Run the test manually by simply calling the binary, and prefix it with SYSTEMD_LOG_LEVEL=debug. Also copy and paste text, not screenshots.
Thank you very much, I am sorry for the screenshots. I will copy and paste next time. The fail is not from my changes. I already pushed, kindly review https://github.com/systemd/systemd/pull/31666. Thank you very much.
Hi @bluca I took time to learn how to set up well, and I'm now running everything well. Here's my submission to repeatable task 1:
Added log message to systemd-notify
Run & Output of tool
I took time to learn how to set up well, and I'm now running everything well. Here's my submission to repeatable task 1:
Looks good, you can proceed to the next step
@bluca While going through the src/test/
directory, I saw some files which do not have test. I specificaly take note of two of them
Do they not require test file ? if not, I am thinking of trying to add a test file to it, kindly give any suggestion on how I can go about it. Thank you.
Yes having more tests is always good, you can look at existing test sources to see how they are integrated in the meson build system
@bluca while going through the issues page https://github.com/systemd/systemd/issues?q=is%3Aopen+is%3Aissue+label%3Aplease-submit-as-pr I realized issue #30836 had been resolved and merged here #31069. But the issue is still visible and hasn't been closed. Could there be something not done yet?
@bluca while going through the issues page https://github.com/systemd/systemd/issues?q=is%3Aopen+is%3Aissue+label%3Aplease-submit-as-pr I realized issue #30836 had been resolved and merged here #31069. But the issue is still visible and hasn't been closed. Could there be something not done yet?
It was done, but just not linked so it wasn't automatically closed, it's been fixed now
@bluca I added unittestcases to expand coverage for the argv_looks_like_help function (located in the src/basic/argv-util.c module) to the test-argv-utils.c file. Please kindly review #31680.
@bluca #31682 is my submission for the short tasks. Kindly let me know of any feedback whenever possible.
Intro
This ticket will serve as the central point tracking the contributions of perspective Outreachy (spring 2024 cohort) applicants.
For those interested, this process is described here: https://www.outreachy.org/apply/make-contributions/ The systemd project will participate again this year: https://www.outreachy.org/outreachy-june-2024-internship-round/communities/systemd/#add-linux-services-live-debugging-functionality-to
A series of tasks will be listed here. Some of them will be "repeatable", ie: some basic tasks that every applicant can do. Some can only be done once, so they are first-come first-served. Some will be relatively quick and should be doable in a couple hours, others might take several days, and they'll be tagged as such.
Every applicant should start with the basic repeatable tasks, and complete at least two. Simply quote the task in a comment, posting proof of completion. We will "thumbs up" the comment to acknowledge completion.
After at least two of those are done and acked, then applicants should move on to the "non-repeatable" tasks. Write a comment quoting the task you selected, and if it's open-ended specify what exactly you are going to do, before you start. Those should end up with a pull request. Note that simply opening a pull request is not enough, they will be reviewed and might need follow-ups before they can be merged, so take that into account when planning for time.
Applicants should complete and wait for ack on completion at least 2 of the repeatable tasks before moving on to the short/long tasks.
Documentation for new contributors
Contributors who are new to
systemd
are highly encouraged to start from the following documents:
- https://systemd.io/CONTRIBUTING/
- https://systemd.io/CODING_STYLE/
- https://systemd.io/HACKING/
- https://systemd.io/CODE_QUALITY/
- https://systemd.io/ARCHITECTURE/
Repeatable tasks
- clone the project, build it, add an
Hello World from <name>!
log message to a tool of your choice fromsrc/
, run it and post a screenshot as proof- clone the project, build it, pick one tool under
src/
of your choice and run it, inspect the code and in your own word write a paragraph explaining what the code (or a section of the code, some are quite large!) does - note that we don't mean just from the point of view of a user, as that's explained in the manual, but describe the actual code- study the documentation, and write a paragraph summarizing one of the documents in your own words. Pick at least one medium/large size document, OR at least three smaller ones. For easier viewing, documents under
docs/
are rendered on https://systemd.io/ and documents underman/
are rendered on https://www.freedesktop.org/software/systemd/man/Short tasks
- fix one of the issues labelled as "please-submit-as-pr" https://github.com/systemd/systemd/issues?q=is%3Aopen+is%3Aissue+label%3Aplease-submit-as-pr
- improve an existing unit test
src/test/
to increase code coverage for the basic common code - see undersrc/basic/
on https://coveralls.io/github/systemd/systemd- add a new unit test to cover the 'shared' common code - see under
src/shared/
on https://coveralls.io/github/systemd/systemdLong tasks
Please note that the description might sound simple, but in reality the task might require more time than you have available. Before starting, state in a comment which issue/TODO you'd like to tackle and how much time you have available, and we'll try to advise on whether it's enough to complete it.
- Add a set of assertion macros to tests.h (ASSERT_OK(), ASSERT_EQ(), ASSERT_GE(), ...) that log the failed condition before crashing and convert a unit test file to use them
- fix one of the issues labelled as "RFE" https://github.com/systemd/systemd/issues?q=is%3Aopen+is%3Aissue+label%3A%22RFE+%F0%9F%8E%81%22
- fix one of the TODO items https://github.com/systemd/systemd/blob/main/TODO
- add an integration test
test/README.testsuite
for a tool/daemon that is not currently covered
According to the instructions, @bluca, for the short tasks, we are to expand existing unit tests for the src/basic/ directory and write new tests for the src/shared directory. Please are we strictly to write new tests for files without tests only from the src/shared directory or we can also write new tests for those files in src/basic without tests?
Sure, you can add a new test for src/basic modules too
@bluca Submission of my Repeatable Tasks 2 & 3 combined.
Tool: systemd-notify
systemd-notify
allows services to communicate with systemd
to send status updates or notifications. It is particularly useful for signaling the readiness of services or reporting errors during startup, which helps systemd
manage dependencies between services more effectively.
systemd-notify
as in src/notfiy.c
has 5 functions:
int help(void)
terminal_urlify_man
call.
2.pid_t manager_pid(void)
pid_t pid_parent_if_possible(void)
int parse_argv(int argc, char *argv[])
getopt_long
function. It uses a static array of option structures to define the long options and their corresponding short options. Based on the parsed options, it sets various variables and flags to control program behavior, and eventually returns a value to indicate success or failure.int run(int argc, char* argv[])
systemd-notify
man page and some of it's other related tool (sd_notify()
).sd_notify()
Notificationssystemd
uses a mechanism (sd_notify()) that allows services or programs that run in the background to inform systemd
about their status. However, for systemd
to correctly recognize which service a status update is coming from, certain conditions need to be met:
When a background service sends a notification to systemd
and then immediately quits, systemd
may not be able to figure out which service sent the message. This is because systemd
tries to match the notification with an active process. If the process has already exited, systemd
cannot make this match and will ignore the message.
If systemd
started the process itself or if the process's permissions are set up in a specific way (using NotifyAccess=main or NotifyAccess=exec), systemd can track it more effectively. In these cases, even if the process sends a notification and exits, systemd
knows which service it belonged to because it has been keeping track of it from the beginning.
Example: Imagine a service that starts up, performs a quick task, and then sends a "task complete" notification using sd_notify()
before immediately exiting. If systemd
hasn't been set up to track this process correctly, it won't know which service sent the "task complete" message and will ignore it.
systemd-notify
Workssystemd-notify
is a tool that services can use to send these notifications more easily. It tries to make sure systemd
correctly attributes the notification to the right service.
Initially, systemd-notify
tries to send the notification as if it was sent by its parent process (the process that called systemd-notify
). This makes sense when a script uses systemd-notify
because it means the notification appears to come directly from the script rather than from systemd-notify
. However, this requires certain privileges, and if systemd-notify
doesn't have them, this step will fail.
Falling Back to Its Own Process ID - If the above doesn't work, systemd-notify
will then send the notification using its own process ID.
The --pid= option can be used to control this behavior, allowing you to specify exactly which process ID (systemd-notify
should pretend to be.
LMK if I can proceed to the short tasks.
LMK if I can proceed to the short tasks.
Looks good, you can proceed
@bluca, for my short tasks(fix one of the issues labeled as "please-submit-as-pr"), I decided to work on #20625
The creator suggests a feature for systemd
to allow central logging to the host namespace for units running in containers (lxc) with their own systemd-journald
.
The creator of the issue attached a patch-set as POC. I have dug into the set to understand the proposed patches and how to merge them into a PR. However, before attempting to connect the patches, I wanted to highlight the patches and my understanding of them and ask for your guidance on how I could best proceed.
Here are the summaries of each proposed patch and some code to be added/modified. Please LMK
Introduces HOSTONLY_JOURNALD
definition in (src/basic/log.c). The definition is used to determine a couple of additional headers and checks. i.e.,virt.h
, and two variables and conditions to check if the systemd-journald
process is running in containers. If it is in a container, the prohibit_ipc
flag is ignored with the expectation that connection is established to host namespace journals.
This patch mainly focus on modifying (/src/journal/journald-server.c) to add unit attribute to the container unit logs. This is logical because in someway the logs from container units have to be identified as from a source.
As such, the changes add the unit name artificially (determined from the unit's group). if only, the HOSTONLY_JOURNALD
is defined! It does so by adding the following condition:
changes:
+#ifdef HOSTONLY_JOURNALD
+ if (c->unit == NULL) {
+ if (c->cgroup != NULL && strstr(c->cgroup, "/lxc.payload") != NULL) {
+ char *name = strrchr(c->cgroup, '/');
+ if (name) {
+ IOVEC_ADD_STRING_FIELD(iovec, n, name+1, "_SYSTEMD_UNIT");
+ }
+ }
+ } else {
+#endif
+ IOVEC_ADD_STRING_FIELD(iovec, n, c->unit, "_SYSTEMD_UNIT"); /* Unit names are bounded by UNIT_NAME_MAX */
+#ifdef HOSTONLY_JOURNALD
+ }
+#endif
Modifies libsystemd's sd_id128_get_boot
function to return the host boot_id. The patch modifies the sd_id128_get_boot function to try to use the host's boot_id across containers. This would make all journal entries, regardless of whether they're generated on the host or within containers, appear as if they come from the same boot session when viewed from the host's perspective.
The patch introduces a conditional compilation block with #ifdef HOSTONLY_JOURNALD
, which suggests that this code path is only included in builds where a single systemd-journald instance is meant to handle logs for both the host and containers.
It attempts to read the boot_id from a new location (HOST_BOOTID_PATH), which is presumably a path to where the host's boot_id is made available (bind mounted) within the container's filesystem.
If reading from HOST_BOOTID_PATH fails (either because the file does not exist or is not accessible), it falls back to the existing method of reading the boot_id directly from the container's /proc/sys/kernel/random/boot_id.
changes:
+#ifdef HOSTONLY_JOURNALD
+ r = id128_read(HOST_BOOTID_PATH, ID128_UUID, &saved_boot_id);
+ if (r < 0) {
+#endif
+ r = id128_read("/proc/sys/kernel/random/boot_id", ID128_UUID, &saved_boot_id);
+ if (r < 0)
+ return r;
+#ifdef HOSTONLY_JOURNALD
+ }
+#endif
The patch defines a new configuration option, HOST_BOOTID_PATH, specifying the filesystem path at which the host's boot_id is made available to containers. This path is presumably used by containers to access the host's boot_id, ensuring that logs from containers are tagged with the host's boot session identifier, thus aligning with the changes discussed in the previous patch.
Changes:
+conf.set_quoted('HOST_BOOTID_PATH', 'run/systemd/journal/host_boot_id')
+conf.set('HOSTONLY_JOURNALD', 1)
Good day @bluca kindly review my latest task #31693.
for my short tasks(fix one of the issues labeled as "please-submit-as-pr"), I decided to work on #20625
Please avoid that, it was marked by mistake
Greetings @bluca , here is my first repeatable task. Tool: backlight
Output:
Greetings @bluca , here is my first repeatable task.
Looks good, please go ahead with the next step
Hello @bluca sir. This is my repeatable task 1:
Hello @bluca sir. This is my repeatable task 1
Looks good, you may go ahead with the next
Hello @bluca can I start with the long task after the merge of ? If yes, I want to start with the first one, what is the target number of macros that we want to add? We have three named above, I guess ASSERT_LE() is also another one that we can add.
I am thinking of having something similar to this.
#define ASSERT_OK(expr) \
do {
if (!(expr)) {
fprintf(stderr, "Assertion failed: %s\n", #expr);
abort();
}
} while (false)
Is this okay ?
Also are we doing it for all the tests on the repository ? Thank you. If there is also any other information that can help, kindly share. Thank you very much once again.
Yes, you can start on that. Use logging and braced groups. The details can be discussed in the PR. It is enough to start with one test file being converted.
Good evening @bluca. After my merges, Please for the long tasks, are we supposed to do it in the order in which they appear? if no, i want to do an issue in the todo list which involves making chase() return O_DIRECTORY if the path has a trailing slash. and also refusing resolution if the input has a trailing slash but the final node isn't a directory.
No ordering constraints, you can pick any that makes sense
okay thank you
Intro
This ticket will serve as the central point tracking the contributions of perspective Outreachy (spring 2024 cohort) applicants.
For those interested, this process is described here: https://www.outreachy.org/apply/make-contributions/ The systemd project will participate again this year: https://www.outreachy.org/outreachy-june-2024-internship-round/communities/systemd/#add-linux-services-live-debugging-functionality-to
A series of tasks will be listed here. Some of them will be "repeatable", ie: some basic tasks that every applicant can do. Some can only be done once, so they are first-come first-served. Some will be relatively quick and should be doable in a couple hours, others might take several days, and they'll be tagged as such.
Every applicant should start with the basic repeatable tasks, and complete at least two. Simply quote the task in a comment, posting proof of completion. We will "thumbs up" the comment to acknowledge completion.
After at least two of those are done and acked, then applicants should move on to the "non-repeatable" tasks. Write a comment quoting the task you selected, and if it's open-ended specify what exactly you are going to do, before you start. Those should end up with a pull request. Note that simply opening a pull request is not enough, they will be reviewed and might need follow-ups before they can be merged, so take that into account when planning for time.
Applicants should complete and wait for ack on completion at least 2 of the repeatable tasks before moving on to the short/long tasks.
Documentation for new contributors
Contributors who are new to
systemd
are highly encouraged to start from the following documents:Repeatable tasks
Hello World from <name>!
log message to a tool of your choice fromsrc/
, run it and post a screenshot as proofsrc/
of your choice and run it, inspect the code and in your own word write a paragraph explaining what the code (or a section of the code, some are quite large!) does - note that we don't mean just from the point of view of a user, as that's explained in the manual, but describe the actual codedocs/
are rendered on https://systemd.io/ and documents underman/
are rendered on https://www.freedesktop.org/software/systemd/man/Short tasks
There is no specific order in which these tasks have to be picked, choose any.
src/test/
to increase code coverage for the basic common code - see undersrc/basic/
on https://coveralls.io/github/systemd/systemdsrc/shared/
on https://coveralls.io/github/systemd/systemdLong tasks
Please note that the description might sound simple, but in reality the task might require more time than you have available. Before starting, state in a comment which issue/TODO you'd like to tackle and how much time you have available, and we'll try to advise on whether it's enough to complete it. There is no specific order in which these tasks have to be picked, choose any.
Add a set of assertion macros to tests.h (ASSERT_OK(), ASSERT_EQ(), ASSERT_GE(), ...) that log the failed condition before crashing and convert a unit test file to use themtest/README.testsuite
for a tool/daemon that is not currently covered