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

File name too long when testing image with bazel #183

Closed
Monnoroch opened this issue Nov 23, 2018 · 8 comments · Fixed by #360
Closed

File name too long when testing image with bazel #183

Monnoroch opened this issue Nov 23, 2018 · 8 comments · Fixed by #360

Comments

@Monnoroch
Copy link

I'm using this tool with bazel's container_test rule from here. Here is the error I'm getting:

Error: error creating driver: processing tar image reference: mkdir /tmp/root.cachebazel_bazel_rootdce6ed4f9b6b93214722cd6fa27af714execrootastrabazel-outk8-fastbuildbinsomeveryveryveryveryveryveryveryveryveryveryverylongname_test.runfilessomeveryveryveryveryveryveryveryveryveryveryverylongname.tar@sha256:27157e6b7e4373f939de9c51191487f9cbf80e6e7f132445fa5a2d605c3e39fb691144971: file name too long

The error doesn't occur on 1.4 and 1.5, only on 1.6, so I'm guessing something has changed.

@nkubala
Copy link
Contributor

nkubala commented Dec 4, 2018

Hey @Monnoroch, thanks for the issue. v1.6 refactored container-structure-test to use new container-diff code that lets go-containerregistry handle the processing of the tar image, and then unpacks that image reference on disk. I'm assuming you're running into the Unix file length limit of 255 characters here, so I'm not really sure how this was working before. Is this something you need to support? We could naively truncate the provided file name to 255 characters, but then we could end up with file collisions so I'm hesitant to do that.

@Monnoroch
Copy link
Author

Indeed I'm hitting the Unix file name length issue. I'm using this tool with Bazel's container_test rule, so I can't control the name, it's all in the rules. Basically, the v1.6 is unusable with Bazel (0.19.0) because of this issue. The fact that older versions do work means that there's a change in this tool.

Of course, truncating file names will not work, as Bazel requires you to create pre-declared artifacts with names fixed at analysis time.

I'm not sure who's to blame here, but I figured that maybe you collaborate with the bazelbuild/docker_rules team and can resolve the problem together. Let me know if I can help.

@nkubala
Copy link
Contributor

nkubala commented Dec 4, 2018

It looks to me like that rule is still using v1.4, unless I'm missing something?

Good point about not being able to just truncate the name, though I'm sad we can't just do that and call it a day. I'm not entirely sure the right fix here, since I suspect it might involve some Bazel magic which wouldn't live here. Maybe ping them and see if they have any insight here? I'm happy to make/accept the changes necessary here if we can come up with a clear path forward.

@Monnoroch
Copy link
Author

It does, but I've overriden it and tested it with 1.5 and 1.6 while trying to fix #184.

@Monnoroch
Copy link
Author

@nkubala I think I found the problem. It's this line. In particular, strings.Replace(name, "/", "", -1) creates a super long file name if you have a long path. Also, this line is not portable to windows.

@alexeagle
Copy link
Collaborator

Note, I hit this with rules_oci as well: https://github.com/bazel-contrib/rules_oci/actions/runs/4865585708/jobs/8676166460 - and we haven't been able to enable windows testing on this repo under Bazel: https://github.com/GoogleContainerTools/container-structure-test/blob/main/.github/workflows/bazel.yaml#L16

alexeagle added a commit to aspect-forks/container-structure-test that referenced this issue May 4, 2023
This causes us to hit GoogleContainerTools#183 in most cases.
See https://buildkite.com/bazel/bcr-presubmit/builds/1247#0187e4cc-44e1-4e21-89a7-08e71f612bfc as an example.

We'll need to fix this in order to properly support Windows, but that is already broken for other reasons.
alexeagle added a commit to aspect-forks/container-structure-test that referenced this issue May 4, 2023
This causes us to hit GoogleContainerTools#183 in most cases.
See https://buildkite.com/bazel/bcr-presubmit/builds/1247#0187e4cc-44e1-4e21-89a7-08e71f612bfc as an example.

We'll need to fix this in order to properly support Windows, but that is already broken for other reasons.
loosebazooka pushed a commit that referenced this issue May 4, 2023
* fix(bazel): don't give absolute path for images

This causes us to hit #183 in most cases.
See https://buildkite.com/bazel/bcr-presubmit/builds/1247#0187e4cc-44e1-4e21-89a7-08e71f612bfc as an example.

We'll need to fix this in order to properly support Windows, but that is already broken for other reasons.

* fix: restore an argument which was lost

See bazel-contrib/rules_oci@614eb02#diff-05ca379f64e0094d8769326c0c6f3b42652556d4dd9dc35e117bcaf79bb936bfL35
@alexeagle
Copy link
Collaborator

@Monnoroch I fixed that line 5 years later :)
GoogleContainerTools/container-diff#395

@Monnoroch
Copy link
Author

Monnoroch commented Jul 27, 2023

Thanks @alexeagle! You mught also want to fix #184 before updating GCT-diff to make this rule set compatible with remote execution. I remember spending a really long time figuring out why one cache works and the other one doesn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@alexeagle @donmccasland @Monnoroch @nkubala and others