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

BUG: Incorrect copy of command context from parent to child (trivial fix) #2193

Open
AtricoSoftware opened this issue Oct 1, 2024 · 1 comment

Comments

@AtricoSoftware
Copy link

AtricoSoftware commented Oct 1, 2024

Found in cobra v1.8.1 (latest at time of writing)

Reproduce:
I execute the root command with a context ( ExecuteContext(ctx) ), my arguments resolve to a sub command and the context is passed down to the executing command- CORRECT
I execute this again with a different context. This time the context is not passed and I get the previous context. - INCORRECT

The context is changed in the root command so if my args indicated the root command, this command would execute with the different context as expected. This means that the behaviour is inconsistent with sub commands compared to the root command

I think the problem lies in command.go at lines 1111-1115

// We have to pass global context to children command
// if context is present on the parent command.
if cmd.ctx == nil {
    cmd.ctx = c.ctx
}

cmd is the execute command and c is the root
The check is for context present on the child, not the parent as stated in the comment

The code should read

// We have to pass global context to children command
// if context is present on the parent command.
if c.ctx != nil {
    cmd.ctx = c.ctx
}

Consider if there should be any check at all, whatever context the root has, it was passed with the execute command and is the context expected in the executing command. Even if nil, it should replace the child?

@AtricoSoftware AtricoSoftware changed the title BUG: Incorrect copy of command context from parent to child BUG: Incorrect copy of command context from parent to child (trivial fix) Oct 1, 2024
@AtricoSoftware
Copy link
Author

AtricoSoftware commented Oct 2, 2024

After checking the code, the check for a parent context is unnecessary, a previous check ensures that this cannot be nil
Re-committed with no check
PR: #2194

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

No branches or pull requests

1 participant