Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Sep 27, 2023

This turns a test failure on Linux when running the test as root, but without the LINUX_IMMUTABLE capability, into an early return, with a suggestion to turn on LINUX_IMMUTABLE next time (if possible).

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 27, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK pinheadmz, jonatack

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #28116 (test: update tool_wallet coverage for unexpected writes to wallet by jonatack)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Copy link
Member

@pinheadmz pinheadmz left a comment

Choose a reason for hiding this comment

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

utACK fa40b3e

@maflcko maflcko added this to the 26.0 milestone Sep 28, 2023
@maflcko
Copy link
Member Author

maflcko commented Sep 29, 2023

rfm or is anything left to be done here?

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK fa40b3e

@@ -45,6 +46,11 @@ def reindex_readonly(self):
self.log.warning(f"stdout: {e.stdout}")
if e.stderr:
self.log.warning(f"stderr: {e.stderr}")
if os.getuid() == 0:
Copy link
Member

Choose a reason for hiding this comment

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

if you retouch, maybe

Suggested change
if os.getuid() == 0:
if os.getuid() == 0: # 0 implies user is root (Unix/Linux)

self.log.warning("Return early on Linux under root, because chattr failed.")
self.log.warning("This should only happen due to missing capabilities in a container.")
self.log.warning("Make sure to --cap-add LINUX_IMMUTABLE if you want to run this test.")
return
Copy link
Member

Choose a reason for hiding this comment

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

Question: what does the return here add?

Copy link
Member Author

Choose a reason for hiding this comment

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

return means that the function terminates and returns its return value.

As explained in the pull request title, the return is required to avoid the test failure.

@fanquake fanquake merged commit f66af92 into bitcoin:master Oct 2, 2023
@maflcko maflcko deleted the 2309-test-fix-root-cap- branch October 5, 2023 11:56
Frank-GER pushed a commit to syscoin/syscoin that referenced this pull request Oct 5, 2023
…ap-add LINUX_IMMUTABLE

fa40b3e test: Avoid test failure on Linux root without cap-add LINUX_IMMUTABLE (MarcoFalke)

Pull request description:

  This turns a test failure on Linux when running the test as `root`, but without the `LINUX_IMMUTABLE` capability, into an early return, with a suggestion to turn on `LINUX_IMMUTABLE` next time (if possible).

ACKs for top commit:
  pinheadmz:
    utACK fa40b3e
  jonatack:
    ACK fa40b3e

Tree-SHA512: d986ff8aeae5f8267c21a23d5be16f7c5a4d4d3be045a6999d8b39c7b8672cfe915dedde762cc9965cdc4970940bffc4b0d1412833d8036d4425450eb6181f67
@bitcoin bitcoin locked and limited conversation to collaborators Oct 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants