Skip to content

stdin is not guaranteed to be defined#3609

Merged
oprypin merged 1 commit intomkdocs:masterfrom
blast-hardcheese:patch-1
Apr 17, 2024
Merged

stdin is not guaranteed to be defined#3609
oprypin merged 1 commit intomkdocs:masterfrom
blast-hardcheese:patch-1

Conversation

@blast-hardcheese
Copy link
Contributor

@blast-hardcheese blast-hardcheese commented Mar 25, 2024

In certain constrained environments, stdin will not be defined:

bash -c $'0>&- python -c \'import sys; print(f"stdin: {sys.stdin}")\''
stdin: None

An example of this could be a CI or Deployments environment.

@blast-hardcheese
stdin is not guaranteed to be defined
f0b6749 
In certain constrained environments, stdin will not be defined:
```shell
bash -c $'0>&- python -c \'import sys; print(f"stdin: {sys.stdin}")\''
stdin: None
```
An example of this could be a CI or Deployments environment.
@pawamoy
Copy link
Contributor

pawamoy commented Mar 27, 2024 •
edited
Loading

Thanks, it sounds reasonable to prevent crashes when stdin is closed. However, what does that mean in term of logic? With your change, there will simply be no config loaded? Have you tested it? Shouldn't we still error out (maybe with a more useful message), as I believe a config is needed for MkDocs to work?

Can you also tell us exactly what error you encountered?

@pawamoy
Copy link
Contributor

pawamoy commented Mar 27, 2024 

% bash -c 'echo | 0>&- mkdocs serve -f -'
Traceback (most recent call last):
  File "/home/pawamoy/.local/bin/mkdocs", line 8, in <module>
    sys.exit(cli())
             ^^^^^
  File "/home/pawamoy/.local/pipx/venvs/mkdocs/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pawamoy/.local/pipx/venvs/mkdocs/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/home/pawamoy/.local/pipx/venvs/mkdocs/lib/python3.11/site-packages/click/core.py", line 1686, in invoke
    sub_ctx = cmd.make_context(cmd_name, args, parent=ctx)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pawamoy/.local/pipx/venvs/mkdocs/lib/python3.11/site-packages/click/core.py", line 943, in make_context
    self.parse_args(ctx, args)
  File "/home/pawamoy/.local/pipx/venvs/mkdocs/lib/python3.11/site-packages/click/core.py", line 1408, in parse_args
    value, args = param.handle_parse_result(ctx, opts, args)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pawamoy/.local/pipx/venvs/mkdocs/lib/python3.11/site-packages/click/core.py", line 2400, in handle_parse_result
    value = self.process_value(ctx, value)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pawamoy/.local/pipx/venvs/mkdocs/lib/python3.11/site-packages/click/core.py", line 2356, in process_value
    value = self.type_cast_value(ctx, value)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pawamoy/.local/pipx/venvs/mkdocs/lib/python3.11/site-packages/click/core.py", line 2344, in type_cast_value
    return convert(value)
           ^^^^^^^^^^^^^^
  File "/home/pawamoy/.local/pipx/venvs/mkdocs/lib/python3.11/site-packages/click/core.py", line 2316, in convert
    return self.type(value, param=self, ctx=ctx)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pawamoy/.local/pipx/venvs/mkdocs/lib/python3.11/site-packages/click/types.py", line 83, in __call__
    return self.convert(value, param, ctx)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pawamoy/.local/pipx/venvs/mkdocs/lib/python3.11/site-packages/click/types.py", line 724, in convert
    f, should_close = open_stream(
                      ^^^^^^^^^^^^
  File "/home/pawamoy/.local/pipx/venvs/mkdocs/lib/python3.11/site-packages/click/_compat.py", line 391, in open_stream
    return get_binary_stdin(), False
           ^^^^^^^^^^^^^^^^^^
  File "/home/pawamoy/.local/pipx/venvs/mkdocs/lib/python3.11/site-packages/click/_compat.py", line 315, in get_binary_stdin
    raise RuntimeError("Was not able to determine binary stream for sys.stdin.")
RuntimeError: Was not able to determine binary stream for sys.stdin.

It looks like Click crashes even before MkDocs anyway.

@blast-hardcheese
Copy link
Contributor Author

blast-hardcheese commented Mar 28, 2024

Ah, the case I was talking about was specifically for mkdocs build, where this could be used in some sort of static site build pipeline:

[nix-shell:/tmp/mdcs/my-project]$ 0>&- mkdocs build
Traceback (most recent call last):
  File "/nix/store/wyxf2g08fcfjjbs4zgyw70xvm85rk1v3-python3.12-mkdocs-1.5.3/bin/.mkdocs-wrapped", line 9, in <module>
    sys.exit(cli())
             ^^^^^
  File "/Users/dstewart/.local/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/dstewart/.local/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/Users/dstewart/.local/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/dstewart/.local/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/dstewart/.local/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/wyxf2g08fcfjjbs4zgyw70xvm85rk1v3-python3.12-mkdocs-1.5.3/lib/python3.12/site-packages/mkdocs/__main__.py", line 283, in build_command
    cfg = config.load_config(**kwargs)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/wyxf2g08fcfjjbs4zgyw70xvm85rk1v3-python3.12-mkdocs-1.5.3/lib/python3.12/site-packages/mkdocs/config/base.py", line 369, in load_config
    if fd is not sys.stdin.buffer:
                 ^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'buffer'

While click may also seem to suffer from this, the issue I was running into was what prompted this PR.

That being said, I think the example you gave made sense, since you were closing stdin before specifically telling click to consume stdin via -f -. Perhaps it can fail more gracefully.

@pawamoy
Copy link
Contributor

pawamoy commented Mar 28, 2024

Right, thanks a lot! That's clear now 🙂

Yeah, so 0>&- mkdocs build definitely should not crash. So I vote to approve this change and merge it 🙂

The 0>&- mkdocs build -f - case can be discussed in another issue. I'll see if Click has a relevant issue already.

@mattppal
Copy link

mattppal commented Apr 4, 2024

If this is approved can we merge? Would be helpful for a project of mine :)

pawamoy
pawamoy previously requested changes Apr 6, 2024
View reviewed changes
Copy link
Contributor

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

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

Could you actually add a comment above line 365 to say that stdin can sometimes be closed, and also add a test to assert that MkDocs doesn't crash when stdin is closed?

@oprypin oprypin added this to the 1.6.0 milestone Apr 9, 2024
oprypin
oprypin approved these changes Apr 16, 2024
View reviewed changes
Copy link
Contributor

@oprypin oprypin left a comment

Choose a reason for hiding this comment

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

Ah well it's a simple enough check.
Btw it's not about stdin being closed, it's about it literally being None. So a bit bizarre that mypy doesn't complain about unchecked usage of sys.stdin.some_attribute

The change makes the following work indeed:

0>&- mkdocs serve

@oprypin oprypin dismissed pawamoy’s stale review April 17, 2024 19:14

Let's move forward with this.. see my comment

@oprypin oprypin merged commit 652813d into mkdocs:master Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@oprypin oprypin oprypin approved these changes

@pawamoy pawamoy pawamoy left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.6.0

Development

Successfully merging this pull request may close these issues.

4 participants

@blast-hardcheese @pawamoy @mattppal @oprypin