MDEV-39126: Feedback plugin to use /etc/os-release#4829

Open

itzanway wants to merge 1 commit intoMariaDB:10.11from

itzanway:mdev-39126-os-release

Contributor

itzanway commented Mar 20, 2026

Description

This PR resolves MDEV-39126 by updating the feedback plugin to use /etc/os-release instead of the obsolete /etc/lsb-release for OS discovery.

As noted in the issue, /etc/lsb-release is outdated (Debian now isolates it to its own package), while /etc/os-release is the modern, portable standard across Linux distributions.

Implementation Details

Primary Check: Modified prepare_linux_info() in plugin/feedback/utils.cc to attempt opening /etc/os-release first.
Parsing: Added logic to parse the PRETTY_NAME= field and safely strip the wrapping double quotes to extract a clean, human-readable distribution name.
Fallback Maintained: To ensure backwards compatibility with older systems that may not have /etc/os-release, the original /etc/lsb-release and wildcard mask (/etc/*-release) checks have been retained as fallbacks.

Testing

Confirmed the C++ syntax and parsing logic.
Relying on Buildbot CI to verify cross-platform compilation and ensure no regressions on older build environments.

itzanway force-pushed the mdev-39126-os-release branch from ff55a8a to 363c513 Compare

March 20, 2026 21:17

gkodinov added the External Contribution label

gkodinov requested changes

View reviewed changes

Member

gkodinov left a comment

Thank you for your contribution! This is a preliminary review.

First of all: please squash your two commits into a single one and add a commit message that's complaint to MariaDB condig standards (you can find the .md file with these in the repository).

Secondly (and optionally, at least for the preliminary review): would you please consider implementing a test for this?

I'd give it a day or two and go about it by checking in mtr if I'm running on a suitable platform (otherwise skip). Then I'd inspect the contents of the https://mariadb.com/docs/server/reference/system-tables/information-schema/information-schema-tables/information-schema-feedback-table and try to find the info from os_release there.

itzanway force-pushed the mdev-39126-os-release branch 2 times, most recently from 3c69304 to 6c8e4fc Compare

March 25, 2026 17:53

Contributor Author

itzanway commented Mar 25, 2026

Thank you for the preliminary review! I have addressed the requested changes:

Squashed Commits: All changes are now cleanly squashed into a single commit (I've also removed the accidental merge commit from the branch history).

Standardized Commit Message: The commit message has been updated to follow the MariaDB coding standards (MDEV ID, description, and Signed-off-by).

MTR Test Implemented: I added an MTR test case (os_release.test and .result) that queries the information_schema.feedback table. Since the actual OS output will vary depending on the host machine, I used --replace_column to mask the value, ensuring the test passes consistently across different platforms.

Let me know if there are any further adjustments needed!

itzanway requested a review from gkodinov

March 26, 2026 09:35

gkodinov reviewed

View reviewed changes

Member

gkodinov left a comment

Thank you for trying to come up with a regression test. It's getting there but it needs a bit more work. Please reach out on Zulip if you need guidance on adding the test.

Looks like there are few artifacts of the merge that need pruning too.

sql/mysqld.cc

Member

gkodinov Mar 26, 2026

please revert these.

sql/slave.cc

Member

gkodinov Mar 26, 2026

please revert the changes to this whole file.

sql/wsrep_mysqld.cc

Member

gkodinov Mar 26, 2026

please revert this whole file.

sql/wsrep_mysqld.h

Member

gkodinov Mar 26, 2026

please revert this whole file.

mysql-test/suite/galera/t/galera_as_slave_replay.cnf

Member

gkodinov Mar 26, 2026

please revert this whole file.

mysql-test/suite/galera/t/MDEV-39011.cnf

Member

gkodinov Mar 26, 2026

please revert this whole file.

mysql-test/suite/galera/t/MDEV-39011.test

Member

gkodinov Mar 26, 2026

please revert this whole file.

mysql-test/suite/galera/r/MDEV-39011.result

Member

gkodinov Mar 26, 2026

please revert this whole file.

mysql-test/suite/feedback/t/os_release.test Outdated

+              --echo # MDEV-39126: Feedback plugin to use /etc/os-release
+              --echo #
+              --replace_column 2 MASKED_OS_INFO

Member

gkodinov Mar 26, 2026

What good is this test? The data you need to test gets masked out. So it could be empty or wrong and no one will notice.

One way to go is to remove the test completely.

You can also fix it by comparing the data to the contents of /etc/os_release where you are going to read it from. mtr can do perl.
But then you will need to make sure you do that only on platforms that have /etc/os-release. How can you tell .. I don't know. Maybe you could do another perl fragment that checks the presence and readability of this file and in case it's not present or readable just skip the whole test.

Also, we customarily prefix test names with the subsystem. Even when in a suite. This is to allow --do-test to pick these up across the board.

mysql-test/suite/feedback/t/os_release.test Outdated

+              --replace_column 2 MASKED_OS_INFO
+              SELECT variable_name, variable_value FROM information_schema.feedback WHERE variable_name = 'os';
+                    No newline at end of file

Member

gkodinov Mar 26, 2026

Please always terminate the last line of a test file.

We also customarily add a

# End of 10.11 tests

at the end of the file.

itzanway force-pushed the mdev-39126-os-release branch from 6c8e4fc to c07be7b Compare

March 26, 2026 11:27

Contributor Author

itzanway commented Mar 26, 2026

I have updated the PR to address your feedback:

Cleaned Artifacts: I've performed a hard reset to the current 10.11 base and selectively applied my changes to ensure all accidental merge artifacts (like the Galera files) are gone.

Single Commit: The PR is now squashed into a single commit following MariaDB coding standards (MDEV ID and Signed-off-by).

Improved MTR Test: I've renamed the test to feedback_os_release.test and replaced the column masking with a dynamic Perl-based check. The test now verifies that the information_schema.feedback table correctly reflects the PRETTY_NAME from /etc/os-release. It also skips gracefully on platforms where the file is missing.

Formatting: Added the # End of 10.11 tests footer and ensured a trailing newline.

Please let me know if any further adjustments are needed!

itzanway requested a review from gkodinov

March 26, 2026 12:54

gkodinov requested changes

View reviewed changes

Member

gkodinov left a comment

The test is shaping up well. Thank you for choosing to work on it!

Some more cleanups to go.

mysql-test/suite/feedback/t/feedback_os_release.test Outdated

    
            @@ -0,0 +1,39 @@
          
              --source include/have_partition.inc

              --perl

Member

gkodinov Mar 26, 2026

I'd write to this file in all cases. Because right now, if there's no os-release there will be no file to include.

We also customarily prefix the temp file names with the mdev number: so that if you see a stray temp file you'd know where to look. I'd name the file e.g. mdev39126_skip_test.inc.

mysql-test/suite/feedback/t/feedback_os_release.test Outdated

    
            @@ -0,0 +1,39 @@
          
              --source include/have_partition.inc

Member

gkodinov Mar 26, 2026

have_partition? why that? The rest of this plugin's tests have --source include/not_embedded.inc

mysql-test/suite/feedback/t/feedback_os_release.test Outdated

    
                  close(OUTPUT);

                }

              EOF

              --source $ENV{MYSQLTEST_VARDIR}/tmp/skip_test.inc

Member

gkodinov Mar 26, 2026

fwiw, mysqltest can reference this directory by just $MYSQLTEST_VARDIR.

Also, we clean up after the test. Please add --remove_file $MYSQLTEST_VARDIR/tmp/skip_test.inc

mysql-test/suite/feedback/t/feedback_os_release.test Outdated

    
              # Get the value from the plugin

              let $os_val = `SELECT variable_value FROM information_schema.feedback WHERE variable_name = 'os'`;

              # Get the expected value from the system via perl

Member

gkodinov Mar 26, 2026

fwiw, perl can access mtr variables :

let foo=bar;
perl;
    print $ENV{'bar'};

So you could do your comparison in perl and avoid the file altogether.

mysql-test/suite/feedback/t/feedback_os_release.test Outdated

    
                  }

                }

              EOF

              --source $ENV{MYSQLTEST_VARDIR}/tmp/os_val.inc

Member

gkodinov Mar 26, 2026

if you are keeping the temp file, please remove it after.

mysql-test/suite/feedback/t/feedback_os_release.test Outdated

    
              EOF

              --source $ENV{MYSQLTEST_VARDIR}/tmp/skip_test.inc

              --echo # MDEV-39126: Feedback plugin to use /etc/os-release

Member

gkodinov Mar 26, 2026

move this at the top of the file please. otherwise it looks like the perl thing above is not a part of the test for this mdev.

itzanway force-pushed the mdev-39126-os-release branch from c07be7b to 4e3b4b3 Compare

March 27, 2026 10:21


          MDEV-39126: Feedback plugin to use /etc/os-release

7a363d0

* Updated the feedback plugin to use /etc/os-release instead of the
  obsolete /etc/lsb-release for OS discovery.
* Fallback to /etc/lsb-release and /etc/*-release is maintained for
  older systems.
* Added regression tests for os-release parsing.

itzanway force-pushed the mdev-39126-os-release branch from 4e3b4b3 to 7a363d0 Compare

March 27, 2026 10:35

Contributor Author

itzanway commented Mar 27, 2026

Thanks for the feedback, @gkodinov! I've force-pushed an updated, squashed commit. I moved the MDEV header to the top, swapped the include file to not_embedded.inc, and made sure the skip file uses the mdev39126 prefix and gets cleaned up properly. I also completely removed the second temp file by passing $os_val directly into the Perl environment for the comparison. Ready for another look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

External Contribution