Add bin/mkosi-initrd and bin/mkosi-sandbox wrappers and some cleanups #3010
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not opposed to the line-length changes, although that's probably something where we should go to 109 in general, since right now they are within the limit of 119.
The generator change in sandbox will probably not fly, since that would probably also pull in Generator from typing, but I'll leave that to @DaanDeMeyer since he did all the import wrangling for sandbox.
mkosi/sandbox.py
Outdated
| @@ -6,6 +6,7 @@ | |||
| itself. To keep the runtime of this script to a minimum, please don't import any extra modules if it can be avoided. | |||
| """ | |||
|
|
|||
| import contextlib | |||
There was a problem hiding this comment.
This stuff is extremely sensitive to ms amount increases given how many times we invoke it. Please drop this, I don't think the drop in complexity outweights the extra import in this case.
There was a problem hiding this comment.
Some non scientific benchmarks to show it does add overhead:
➜ mkosi git:(sandbox) time python3 -S -I -c "import ctypes; import os; import sys;"
________________________________________________________
Executed in 11.17 millis fish external
usr time 7.13 millis 327.00 micros 6.80 millis
sys time 3.95 millis 46.00 micros 3.90 millis
➜ mkosi git:(sandbox) time python3 -S -I -c "import ctypes; import os; import sys; import contextlib"
________________________________________________________
Executed in 20.48 millis fish external
usr time 32.30 millis 15.97 millis 16.33 millis
sys time 7.91 millis 4.87 millis 3.05 millis
➜ mkosi git:(sandbox) time python3 -S -I -c "import ctypes; import os; import sys;"
________________________________________________________
Executed in 11.40 millis fish external
usr time 8.30 millis 370.00 micros 7.93 millis
sys time 3.04 millis 52.00 micros 2.99 millis
➜ mkosi git:(sandbox) time python3 -S -I -c "import ctypes; import os; import sys; import contextlib"
________________________________________________________
Executed in 14.01 millis fish external
usr time 12.56 millis 0.00 micros 12.56 millis
sys time 1.44 millis 387.00 micros 1.05 millis
There was a problem hiding this comment.
I have my doubts whether this microbenchmark is useful. I expect that normally something else pulls in contextlib, so the addition "import" is just a lookup in sys.modules, i.e. effectively free. But it's not terribly important, so I dropped the commit.
There was a problem hiding this comment.
Well this microbenchmark imports everything that's imported by sandbox.py, so there's actually nothing pulling in contextlib. Upstream python goes through quite a bit of effort to keep the imports of core modules like these very minimal
RuntimeError is for "unexpected errors". When the argument has a wrong value, ValueError is the standard exception to use.
It is convenient to be able to invoke those two during development too, just like mkosi itself.
.dir-locals specifies "fill-column" as 99 for .py files. This seems resonable, because then the comments mostly match the general width of the surrounding code.
keszybz
force-pushed
the
mkosi-sandbox-wrapper
branch
from
c1ed59c to
6f753c8
Compare
No description provided.