mollyibot commented 10 months ago

I need to reconsider this approach, because this approach is only compatible with bzlmod not old bazel. with bzlmod, the runfiles directory changing to _main broke various assumptions, previously it was mapped to workspace name.

J2CL is not compatible with bzlmod(not only runfile issue but also some other issues). for repos using old bazel (like j2cl) they probably do not have repo_mapping file genereated in runfile folder and cannot get remapped to _main.

gkdn commented 10 months ago

Is it ready for another round or are you still testing out?

mollyibot commented 10 months ago

It is ready for another round. I think there maybe other options like creating a symlink or trigger repo_mapping to generate for old bazel version. I explore rules_python and their approach is to explore an additional directory. For our case, it is bash script that cannot find the resolved runfile location, the minimal change I think of is that we explore another directory.

bcsgh commented 8 months ago

I'm seeing what seems to be a related error?

==================== Test output for //server:js_test:
../io_bazel_rules_closure/third_party/phantomjs/phantomjs: line 47: /tmp/bazel-working-directory/server/bazel-out/k8-fastbuild/bin/server/js_test.runfiles/tbd_server//third_party/phantomjs/bin/phantomjs: No such file or directory
================================================================================

That doubled // looks interesting. If I look around a bit from just under that:

$ find  bazel-out/k8-fastbuild/bin/server/js_test.runfiles/ -type l -name phantomjs
bazel-out/k8-fastbuild/bin/server/js_test.runfiles/io_bazel_rules_closure/third_party/phantomjs/phantomjs
bazel-out/k8-fastbuild/bin/server/js_test.runfiles/io_bazel_rules_closure/third_party/phantomjs/bin/phantomjs
bazel-out/k8-fastbuild/bin/server/js_test.runfiles/tbd_server/external/io_bazel_rules_closure/third_party/phantomjs/phantomjs
bazel-out/k8-fastbuild/bin/server/js_test.runfiles/tbd_server/external/io_bazel_rules_closure/third_party/phantomjs/bin/phantomjs

It looks like there should be a server/external/io_bazel_rules_closure or just a io_bazel_rules_closure between the two //.

At a guess, I think this branch [line 23] is being hit when the else should happen?

mollyibot commented 8 months ago

Can you give some information on how to reproduce the issue so that I can take a look? can you see if there is anyfile inside _repo_mapping? see this line if it hits line [22]?

bcsgh commented 8 months ago

Unfortunately, it's in some non-public code so a test case will take some work (I might be able to get to that tomorrow.

I did already check for bazel-out/k8-fastbuild/bin/server/js_test.runfiles/../_repo_mapping and it didn't even exist.

One that might be relevant: I'm running with --noenable_bzlmod in my .bazelrc for the time being. I intend to strip that, but not until after my current project ships and opens up the schedule.

mollyibot commented 8 months ago

I see, thank you for your update, our initial code is for --noenable_bzlmod, this change's goal was to incorporate bzlmod, so this is done under bzl mod, but I am very curious if no _repo_mapping folder it should fall under line 25

bcsgh commented 8 months ago

Won't [[ -f some_path ]] check that `some_path is a File not a directory?

Quoting from the man page:

-d file True if file exists and is a directory. -e file True if file exists. -f file True if file exists and is a regular file.

mollyibot commented 8 months ago

sorry for misleading you in previous message, _repo_mapping is a file containing repositoroy mapping information, can you check what is inside _repo_mapping if you have that file?

bcsgh commented 8 months ago

tl;dr; My current theory is that execution is running via line 29.

find bazel-out/ -name phantomjs |
    grep -e '/io_bazel_rules_closure/third_party/phantomjs/phantomjs$' |
    sed 's@$@.runfiles@' |
    ls -1 $(cat)
# ^^^ Finds nothing

find bazel-out/ -name phantomjs |
   grep -e '/io_bazel_rules_closure/third_party/phantomjs/phantomjs$' |
   sed 's@/io_bazel_rules_closure/third_party/phantomjs/phantomjs@/*/@' | 
   ls -d $(cat) |
while read a ;
do 
  if [[ -f "${a}../_repo_mapping" ]] ;
  then 
    echo $a ;
    ls -1d "${a}/third_party/phantomjs/bin/phantomjs" ; 
  # else 
  #   echo $a ;
  #   ls -1d "${a}../io_bazel_rules_closure/third_party/phantomjs/bin/phantomjs" ; 
  fi || echo @@@@ $a  ; 
done

Output:

bazel-out/k8-fastbuild/bin/server/js_test.runfiles/com_google_javascript_closure_library/
ls: cannot access 'bazel-out/k8-fastbuild/bin/server/js_test.runfiles/com_google_javascript_closure_library//third_party/phantomjs/bin/phantomjs': No such file or directory
@@@@ bazel-out/k8-fastbuild/bin/server/js_test.runfiles/com_google_javascript_closure_library/
bazel-out/k8-fastbuild/bin/server/js_test.runfiles/io_bazel_rules_closure/
bazel-out/k8-fastbuild/bin/server/js_test.runfiles/io_bazel_rules_closure//third_party/phantomjs/bin/phantomjs
bazel-out/k8-fastbuild/bin/server/js_test.runfiles/tbd_server/
ls: cannot access 'bazel-out/k8-fastbuild/bin/server/js_test.runfiles/tbd_server//third_party/phantomjs/bin/phantomjs': No such file or directory
@@@@ bazel-out/k8-fastbuild/bin/server/js_test.runfiles/tbd_server/

Explanation

Working backwards from the two paths listed in the error message:

$RUNFILES=/tmp/bazel-working-directory/server/bazel-out/k8-fastbuild/bin/server/js_test.runfiles/tbd_server/
$0=../io_bazel_rules_closure/third_party/phantomjs/phantomjs

From digging around under ./bazel-bin, it looks like to make $0 resolve correctly $PWD must be one of:

./bazel-out/k8-fastbuild/bin/server/js_test.runfiles/*
./bazel-out/k8-fastbuild/bin/server/js_test.runfiles/tbd_server/external/*
./bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/*

Therefor line 21 checks for ../io_bazel_rules_closure/third_party/phantomjs/phantomjs.runfiles which doesn't exist under any of the ${PWD} assumptions and drops to line 28.

Line 28 checks for ${PWD}/../_repo_mapping which does exist in multiple of the ./bazel-out/k8-fastbuild/bin/server/js_test.runfiles/* options and not under any of the others. On to line 29. (Where ${PWD}/../_repo_mapping doesn't exist (line 31), nothing interesting happens.)

Three options go to line 29:

bazel-out/k8-fastbuild/bin/server/js_test.runfiles/io_bazel_rules_closure/ (works)
bazel-out/k8-fastbuild/bin/server/js_test.runfiles/com_google_javascript_closure_library/ (fails, but likely isn't the relevant case)
bazel-out/k8-fastbuild/bin/server/js_test.runfiles/tbd_server/ (FAILS) via line 29

That 3rd case sounds right given that matches the error message. Hunting around, for a third_party/phantomjs/bin/phantomjs, I find two options for line 29 that would make stuff work down on line 47:

RUNFILES="${PWD}/external/io_bazel_rules_closure"
RUNFILES="${PWD}/../io_bazel_rules_closure" (This is basically line 31)

Both of those would seem to work for the other uses of ${RUNFILES}. Should line 28 just unconditionally become line 31? Or maybe the things that's being checked for on 28 is wrong? Should this just be a list of exists checks?

if [[ -d "$0.runfiles" && -f "$0.runfiles/../_repo_mapping" && -d "$0.runfiles/third_party" ]]; then
    RUNFILES="$0.runfiles"
elif [[ -d "$0.runfiles" && -d "$0.runfiles/io_bazel_rules_closure/third_party" ]]; then 
    RUNFILES="$0.runfiles/io_bazel_rules_closure"
elif  [[ -f "${PWD}/../_repo_mapping" && -d "${PWD}/third_party" ]]; then
    RUNFILES="${PWD}"
elif  [[ -d "${PWD}/../io_bazel_rules_closure/third_party" ]]; then
    RUNFILES="${PWD}/../io_bazel_rules_closure"
else
   echo Failed to find RUNFILES relative to ${PWD} or $(dirname $0) 
   exit 1
fi

BTW: damn you; there went my morning: https://xkcd.com/356/ ;0)

The contents of _repo_mapping FWIW looks like it's not garbage or anything:

args4j,server,server
bazel_tools,__main__,server
com_google_absl,server,server
com_google_auto_value,server,server
com_google_auto_value_annotations,server,server
com_google_closure_stylesheets,server,server
com_google_code_findbugs_jsr305,server,server
com_google_code_gson,server,server
com_google_dagger,server,server
com_google_dagger_compiler,server,server
com_google_dagger_producers,server,server
com_google_dagger_spi,server,server
com_google_errorprone_error_prone_annotations,server,server
[...]

mollyibot commented 8 months ago

This looks like a hacky way to check it, is your workspace named server? and I feel this bazel-out/k8-fastbuild/bin/server/js_test.runfiles/server/external/io_bazel_rules_closure/third_party/phantomjs/bin/phantomjs should be found based on the current condition, not bazel-out/k8-fastbuild/bin/server/js_test.runfiles/tbd_server/third_party/phantomjs/bin/phantomjs usually the runfile would be stored in $0.runfiles., this is really where it get mapped to server rather than tbd_server

bcsgh commented 8 months ago

Both the project and the main directory were done as "server". (Well, they were after I scrubbed some internal data.) I've tried to edit the project to tbd_server.

Actually, I just realized that project is public (somehow I was thinking it was a different project?): https://github.com/bcsgh/tbd-http

tl;dr; I haven't tried this, but I think I've managed to copy paste all the needed parts below:

`WORKSPACE`

workspace(name = "tbd_server")

load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")

git_repository(
    name = "com_google_protobuf",
    remote = "https://github.com/protocolbuffers/protobuf.git",
    commit = "08ec050c44c3ffe3074f0e14a4a47095d14dd855",  # current as of 2023/11/06
    shallow_since = "1699282866 -0800",
)

git_repository(
    name = "io_bazel_rules_closure",
    remote = "https://github.com/bazelbuild/rules_closure.git",
    commit = "ac78ac89f554e8b18c497d306832dbb86f51087a",
    # shallow_since = "1697673471 -0700",
)

load("@io_bazel_rules_closure//closure:repositories.bzl", "rules_closure_dependencies", "rules_closure_toolchains")
rules_closure_dependencies(
    omit_bazel_skylib=True,
    omit_com_google_protobuf=True,
    omit_rules_cc=True,
)
rules_closure_toolchains()

git_repository(
    name = "rules_cc",
    commit = commit or "2f8c04c04462ab83c545ab14c0da68c3b4c96191",  # current as of 2022/06/22
    remote = "https://github.com/bazelbuild/rules_cc.git",
    shallow_since = "1655902949 -0700",
)

git_repository(
    name = "bazel_skylib",
    commit = commit or "9c9beee7411744869300f67a98d42f5081e62ab3",  # current as of 2023/11/12
    remote = "https://github.com/bazelbuild/bazel-skylib.git",
    shallow_since = "1699201005 -0500",
)

`//server:BUILD`

load("@io_bazel_rules_closure//closure/testing:closure_js_test.bzl", "closure_js_test")
[...]
closure_js_test(
    name = "js_test",
    srcs = ["test.js"],
    entry_points = ["server/test.js"],
    suppress = ["moduleLoad"],
    deps = [
        ":js",
        "@com_google_javascript_closure_library//closure/goog/testing:asserts",
        "@com_google_javascript_closure_library//closure/goog/testing:jsunit",
        "@com_google_javascript_closure_library//closure/goog/testing:testsuite",
    ],
)
[...]

bcsgh commented 8 months ago

Side note: there are some interesting similarities between this issue and another one in a different project: https://github.com/bazelbuild/rules_pkg/issues/803

The solution might not be the same across both, but if I had to guess they might be mutually informative.

bazelbuild / rules_closure

Update web testing version and resolve runfile path after bazel enable bzlmod by default #597

Explanation

`WORKSPACE`

`//server:BUILD`