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

lib/utmp.c: Use the appropriate autotools macros for struct utmpx #955

Merged
merged 5 commits into from
Feb 21, 2024

Conversation

alejandro-colomar
Copy link
Collaborator

Oops. I forgot to do a change.

Recently, we started using utmpx instead of utmp, and we updated
<./configure.ac> to do the checks for 'struct utmpx' instead of
'struct utmp'.  However, I forgot to update the preprocessor
conditionals accordingly.

Fixes: 64bcb54 ("lib/, src/, configure.ac: Use utmpx instead of utmp")
Link: #954
Cc: @firasuke
Cc: @awilfox
Cc: @ikerexxe

@alejandro-colomar
Copy link
Collaborator Author

v2 changes:

  • Remove #endif comments in a separate commit.
$ git range-diff shadow/master gh/utmpx utmpx 
1:  8ca977f9 = 1:  8ca977f9 lib/utmp.c: Indent nested preprocessor conditionals
2:  ee54d49b = 2:  ee54d49b lib/utmp.c: Merge preprocessor conditionals
3:  e4b4664c ! 3:  1bc3b340 lib/utmp.c: Use defined() instead of #if[n]def
...
-:  -------- > 4:  7c7223df lib/utmp.c: Use defined() instead of #if[n]def
4:  5d9b0245 = 5:  fe88a9a0 lib/utmp.c: Use the appropriate autotools macro for struct utmpx

$ git diff 5d9b0245..fe88a9a0
$ 

@alejandro-colomar
Copy link
Collaborator Author

v2b changes:

  • Use plural in commit subject line
$ git range-diff shadow/master gh/utmpx utmpx 
1:  8ca977f9 = 1:  8ca977f9 lib/utmp.c: Indent nested preprocessor conditionals
2:  ee54d49b = 2:  ee54d49b lib/utmp.c: Merge preprocessor conditionals
3:  1bc3b340 = 3:  1bc3b340 lib/utmp.c: Remove #endif comments
4:  7c7223df = 4:  7c7223df lib/utmp.c: Use defined() instead of #if[n]def
5:  fe88a9a0 ! 5:  52b4deef lib/utmp.c: Use the appropriate autotools macro for struct utmpx
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/utmp.c: Use the appropriate autotools macro for struct utmpx
    +    lib/utmp.c: Use the appropriate autotools macros for struct utmpx
     
         Recently, we started using utmpx instead of utmp, and we updated
         <./configure.ac> to do the checks for 'struct utmpx' instead of

@alejandro-colomar alejandro-colomar changed the title lib/utmp.c: Use the appropriate autotools macro for struct utmpx lib/utmp.c: Use the appropriate autotools macros for struct utmpx Feb 20, 2024
@alejandro-colomar alejandro-colomar marked this pull request as ready for review February 20, 2024 18:31
This was referenced Feb 20, 2024
Copy link
Collaborator

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

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

LGTM!

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Feb 21, 2024 via email

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Indentation makes it clear which is which.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Recently, we started using utmpx instead of utmp, and we updated
<./configure.ac> to do the checks for 'struct utmpx' instead of
'struct utmp'.  However, I forgot to update the preprocessor
conditionals accordingly.

Fixes: 64bcb54 ("lib/, src/, configure.ac: Use utmpx instead of utmp")
Link: <shadow-maint#954>
Cc: Firas Khalil Khana <firasuke@gmail.com>
Cc: "A. Wilfox" <https://github.com/awilfox>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Collaborator Author

I'm not able to reproduce the problem from the CI, so I'll open a PR to be able to see those errors in the CI.

@alejandro-colomar
Copy link
Collaborator Author

I've put the commit in this PR, so that we can test it with this failure.

v3 changes:

  • Use trap(1) in CI to show the testsuite.log

(I'll probably want to do something similar with the cmocka logs.)

$ git range-diff shadow/master gh/utmpx utmpx 
1:  bceeefe2 = 1:  bceeefe2 lib/utmp.c: Indent nested preprocessor conditionals
2:  91e43fee = 2:  91e43fee lib/utmp.c: Merge preprocessor conditionals
3:  3947d9c6 = 3:  3947d9c6 lib/utmp.c: Remove #endif comments
4:  08dff703 = 4:  08dff703 lib/utmp.c: Use defined() instead of #if[n]def
5:  5a936adc = 5:  5a936adc lib/utmp.c: Use the appropriate autotools macros for struct utmpx
-:  -------- > 6:  f232202e .github/workflows/runner.yml: Use trap(1) to show the testsuite log

@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Feb 21, 2024

Huh, a Heisenbug. When we try to measure it, it disappears. :|

I'll drop the last patch, and trigger a new run of CI, and see it it reproduces again.

@alejandro-colomar
Copy link
Collaborator Author

v3.1:

  • Drop the last patch, to see if it brings back the Heisenbug.
$ git range-diff shadow/master gh/utmpx utmpx 
1:  bceeefe2 = 1:  bceeefe2 lib/utmp.c: Indent nested preprocessor conditionals
2:  91e43fee = 2:  91e43fee lib/utmp.c: Merge preprocessor conditionals
3:  3947d9c6 = 3:  3947d9c6 lib/utmp.c: Remove #endif comments
4:  08dff703 = 4:  08dff703 lib/utmp.c: Use defined() instead of #if[n]def
5:  5a936adc = 5:  f29895f0 lib/utmp.c: Use the appropriate autotools macros for struct utmpx
6:  f232202e < -:  -------- .github/workflows/runner.yml: Use trap(1) to show the testsuite log

@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Feb 21, 2024

v3.3:

  • Roll back to v2c (another attempt to trigger the heisenbug).

@alejandro-colomar
Copy link
Collaborator Author

Since tests pass now, and we're again in v2c, as reviewed by Iker, I'll merge.

@alejandro-colomar alejandro-colomar merged commit 1af6b68 into shadow-maint:master Feb 21, 2024
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants