Skip to content
This repository has been archived by the owner on Oct 2, 2023. It is now read-only.

do not flatten file paths in container_image #317

Closed
jmhodges opened this issue Feb 9, 2018 · 11 comments
Closed

do not flatten file paths in container_image #317

jmhodges opened this issue Feb 9, 2018 · 11 comments

Comments

@jmhodges
Copy link
Contributor

jmhodges commented Feb 9, 2018

See https://github.com/bazelbuild/bazel/issues/2176

This is still happening and has now gotten weirder: https://github.com/bazelbuild/bazel/issues/2176#issuecomment-364352176

It would be good for container_image's files to not just be the basename files including in the directory, but to, by default, maintain the full relative file paths.

@pcj
Copy link
Member

pcj commented Feb 25, 2018

I find this troubling also. Is this a bug or a feature?

@pcj
Copy link
Member

pcj commented Apr 6, 2018

Friendly ping. This is a problem for adoption of rules_docker.

@rutsky
Copy link

rutsky commented Nov 6, 2018

Setting strip_prefix in tar_pkg workaround helped in my case (not to ., but to specific directory, as in my layout I needed to strip some prefix):

pkg_tar(
    name = "webfrontend-tar",
    srcs = ["//webfrontend:build"],
    mode = "0o644",
    package_dir = "/usr/share/nginx/html/",
    # Otherwise all directories are flattened:
    # <https://github.com/bazelbuild/rules_docker/issues/317>
    strip_prefix = "webfrontend/build/",
)

container_image(
    name = "webfrontend-image",
    tars = [":webfrontend-tar"],
    base = "@nginx_base//image",
    visibility = ["//visibility:public"],
)

Without strip_prefix = "webfrontend/build/", all files are flattened in webfrontend-tar.

@nlopezgi
Copy link
Contributor

nlopezgi commented Dec 2, 2018

is this still an issue? please reopen if so.

@nlopezgi nlopezgi closed this as completed Dec 2, 2018
@jmhodges
Copy link
Contributor Author

jmhodges commented Dec 2, 2018

It is! I can’t reopen this issue, though

@nlopezgi nlopezgi reopened this Dec 2, 2018
@nlopezgi
Copy link
Contributor

nlopezgi commented Dec 2, 2018

Is the workaround you found using pkg_tar noy working or otherwise problematic?

@jmhodges
Copy link
Contributor Author

jmhodges commented Dec 2, 2018

It’s problematic because it’s undocumented and unexpected. It’s really hard to understand why this happens or how to fix it without lots of googling. It’s another weird thing that you have to tell people about when they just want to try out Bazel (and there’s plenty of them, you know?)

@nlopezgi nlopezgi mentioned this issue Dec 4, 2018
nlopezgi added a commit that referenced this issue Dec 4, 2018
fix #597
fix #598
provide docs for recommended fix for #317
Improve documentation for xx_image rules to make more explicit that args such as env and symlinks will never be added to xx_image and container_image must be used in conjunction with xx_image for these use cases.
@nlopezgi
Copy link
Contributor

nlopezgi commented Dec 4, 2018

I have updated the docs (See PR updating readme) to explain this issue. I'm closing this again as I dont think the fix for this issue is on this repo (i.e., it seems to me the issue is with pkg_tar rule). Please let me know if anyone feels there is more we can do here to reopen.

@nlopezgi nlopezgi closed this as completed Dec 4, 2018
@njlr
Copy link

njlr commented Mar 22, 2022

I find it very strange that an extra rule is required to place files in specific folders in a container_image. It seems like an extremely noisy API to me.

Perhaps something like this should be possible?

container_image(
  name = "my_image",
  base = ":nodejs",
  files = {
    "/home/runner/index.js": "//:some_node_rule",
    "/home/runner/assets/logo.png": "logo.png",
  },
)

where the key to the dictionary is the path inside the image and the value is a source file or rule

@aryeh-looker
Copy link

This is a really huge bug imho. This means, for example, if you have a java_binary target with relative file paths based on other targets, the implementation is no longer correct inside docker. I would strongly suggest fixing -- and would be glad to help if mainters are open to that (with some parameter like flatten that defaults to the old behavior of True to avoid breaking folks).

@aryeh-looker
Copy link

aryeh-looker commented Aug 6, 2022

Okay I see that the docs suggest strip_prefix = ".", oof. this no longer appears to be a valid parameter?

okay, this works data_path = ".",

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

No branches or pull requests

6 participants