Conversation
devsnek
force-pushed
the
sandbox-image-override
branch
from
86ca7b7 to
5536645
Compare
|
If this needs tests, where should they go? |
devsnek
force-pushed
the
sandbox-image-override
branch
from
5536645 to
4ce5e85
Compare
emilyalbini
left a comment
emilyalbini
left a comment
There was a problem hiding this comment.
Thanks for opening the PR! Left a few comments.
|
Regarding tests, I'd add a |
devsnek
force-pushed
the
sandbox-image-override
branch
2 times, most recently
from
12c0f9e to
b0eba00
Compare
src/build.rs
Outdated
| } | ||
|
|
||
| /// Get the path to the source code on the host machine (outside the sandbox). | ||
| pub fn get_source_dir(&self, krate: &Crate) -> Result<PathBuf, Error> { |
There was a problem hiding this comment.
Hmm, I'd still just make the Crate::copy_source_to method public instead of an approach like th is.
There was a problem hiding this comment.
Where does one get the target dir from? Should I also make BuildDirectory::source_dir() public? krate.copy_source_to(&workspace, ???)
There was a problem hiding this comment.
Docs.rs would have to create a temporary dir (for example with the tempfile crate) to extract the source code to.
devsnek
force-pushed
the
sandbox-image-override
branch
2 times, most recently
from
dde39c4 to
e5081cb
Compare
devsnek
force-pushed
the
sandbox-image-override
branch
from
e5081cb to
04e9e6b
Compare
| if size > size_limit { | ||
| return Err(CommandError::SandboxImageTooLarge(size)); | ||
| } | ||
| Some(m.config.digest) |
There was a problem hiding this comment.
I don't see any config or digest field with docker manifest inspect. What version of the docker CLI were you using?
> docker manifest inspect ros:latest | jq .config
null
> docker manifest inspect ros:latest
{
"schemaVersion": 2,
"mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
"manifests": [
{
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
"size": 3042,
"digest": "sha256:ba0d9de623ad01a96d420140fed94cfb8de5e75be31cc33f0592d46e10dcccee",
"platform": {
"architecture": "amd64",
"os": "linux"
}
},
{
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
"size": 3041,
"digest": "sha256:42152a7c40961b39313cd0268a8d0aff483b56e817c784515a5b57cf2f2d1f28",
"platform": {
"architecture": "arm64",
"os": "linux",
"variant": "v8"
}
}
]
}
There was a problem hiding this comment.
Wait, with that version I can't see the config part either.
There was a problem hiding this comment.
> docker --version
Docker version 19.03.8, build afacb8b7f0
So I'm worried this doesn't actually work.
3 participants
rust-lang/docs.rs#1193