[Buildbot] Add script for setting up environment for debug/reproduction - #2278
Conversation
jplehr
left a comment
jplehr
left a comment
There was a problem hiding this comment.
Thanks for putting this together!
I think it would be great if a user could point to a local LLVM tree and then have that mounted into the container as a volume mount. This reduces the need for re-cloning.
The other question that I had: Do these docker commands that the script uses require a certain docker version to be available?
Good idea! I added a new flag
The docker cmds and options are fundamental which should be available for the versions widely used. So I think we don't have to enforce the version here. |
| name = container_name(args, args.target) | ||
|
|
||
| log(f"Removing container {name}") | ||
| run_cmd(["docker", "rm", "-f", name], check=False) |
There was a problem hiding this comment.
Should we check if the docker exists first?
This fails when the container is still running, right?
|
The revision has the following changes to address comment feedback:
|
jplehr
left a comment
jplehr
left a comment
There was a problem hiding this comment.
LG overall just one comment.
| return args.name or f"test-{target}" | ||
| def generated_container_name(target): | ||
| for _ in range(10): | ||
| name = f"test-{target}-{random.randint(0, 99999):05d}" |
There was a problem hiding this comment.
Instead of randint does it make sense to use linux time stamp?
There was a problem hiding this comment.
Yes, I actually thought about it. For a 5-digits id using milliseconds, it means the id will repeat every 100 seconds. Comparing to timestamps, the random number is less likely to collide. I will go ahead to land this PR and file a separate PR for the changes if we prefer time stamp.
2 participants
This PR provides a script to help developers quickly set up a local environment for debugging or reproducing issues surfaced by our upstream buildbots. Updated readme for detailed instructions.
This script:
Tested locally.