google / zetasql

ZetaSQL - Analyzer Framework for SQL
Apache License 2.0
2.32k stars 219 forks source link

Build Failures on Linux #105

Open lbhdc opened 2 years ago

lbhdc commented 2 years ago

Hey, I wanted to give zetasql a try, but I have been running into issues getting it to build.

A fresh clone of the repo and bazel test //... fails on linux (fedora35). There are a confluence of problems that I have attempted but failed to work around.

My System

I am using bazelisk for my bazel installation which looks like it correctly picks bazel4 to build zetasql.

> uname -a
Linux phobos 5.17.4-200.fc35.x86_64 #1 SMP PREEMPT Wed Apr 20 15:37:53 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Problem

When building a fresh clone I get the following error.

> git clone https://github.com/google/zetasql.git
...
> bazel test //...
...
ERROR: /tmp/x/zetasql/zetasql/resolved_ast/BUILD:38:23: Executing genrule //zetasql/resolved_ast:run_gen_resolved_ast_proto failed: (Exit 1): bash failed: error executing command /bin/bash -c ... (remaining 1 argument(s) skipped)
...
ImportError: cannot import name 'Mapping' from 'collections' (/usr/lib64/python3.10/collections/__init__.py)
----------------
Note: The failure of target //zetasql/resolved_ast:gen_resolved_ast (with exit code 1) may have been caused by the fact that it is running under Python 3 instead of Python 2...
...

What I Tried

I am not interested in adding python2 to any of my environments given its eol status, so I have attempted the patching route.

I wrote a patch to bump jinja and markupsave to bump their versions to a version that is no longer using the removed api. This eliminated those errors. I also modified the gen_resolved_ast py_binary rule to force python3.

Those modifications clear up the above errors, but I am not out of the woods yet.

I am currently stuck on an issue with the generated protos that are used by resolved_ast_enums_pb2.py. There seems to be some error with the DESCRIPTOR value. When digging through /bazel-bin its defined, and not none.

ERROR: /tmp/zetasql/java/com/google/zetasql/resolvedast/BUILD:26:23: Executing genrule //java/com/google/zetasql/resolvedast:run_gen_resolved_ast_java failed: (Exit 1): bash failed: error executing command /bin/bash -c ... (remaining 1 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox bash failed: error executing command /bin/bash -c ... (remaining 1 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
Traceback (most recent call last):
  File "/home/<username>/.cache/bazel/_bazel_<username>/7732d869f91304b104183878d18da881/sandbox/linux-sandbox/445/execroot/com_google_zetasql/bazel-out/host/bin/zetasql/resolved_ast/gen_resolved_ast.runfiles/com_google_zetasql/zetasql/resolved_ast/gen_resolved_ast.py", line 54, in <module>
    from zetasql.resolved_ast import resolved_ast_enums_pb2
  File "/home/<username>/.cache/bazel/_bazel_<username>/7732d869f91304b104183878d18da881/sandbox/linux-sandbox/445/execroot/com_google_zetasql/bazel-out/host/bin/zetasql/resolved_ast/gen_resolved_ast.runfiles/com_google_zetasql/zetasql/resolved_ast/resolved_ast_enums_pb2.py", line 21, in <module>
    _RESOLVEDSUBQUERYEXPRENUMS = DESCRIPTOR.message_types_by_name['ResolvedSubqueryExprEnums']
AttributeError: 'NoneType' object has no attribute 'message_types_by_name'

Patch

Here is the patch I am currently working with. The changes to arena_allocator.h are because I ultimately want to use this in cpp20, and it is using an api that has been removed.

diff --git a/bazel/zetasql_deps_step_2.bzl b/bazel/zetasql_deps_step_2.bzl
index 4ba0e96..d0c91fe 100644
--- a/bazel/zetasql_deps_step_2.bzl
+++ b/bazel/zetasql_deps_step_2.bzl
@@ -337,10 +337,9 @@ py_library(
         if not native.existing_rule("jinja"):
             http_archive(
                 name = "jinja",
-                # Jinja release 2.10
-                url = "https://github.com/pallets/jinja/archive/2.10.tar.gz",
-                strip_prefix = "jinja-2.10",
-                sha256 = "0d31d3466c313a9ca014a2d904fed18cdac873a5ba1f7b70b8fd8b206cd860d6",
+                url = "https://github.com/pallets/jinja/archive/3.1.2.tar.gz",
+                strip_prefix = "jinja-3.1.2/src",
+                sha256 = "ecae76cd1a064d40eb46c5375f07953d747f4d65b68cd3fa5f02c91714b799fc",
                 build_file_content = """py_library(
     name = "jinja2",
     visibility = ["//visibility:public"],
@@ -348,7 +347,6 @@ py_library(
     deps = ["@markupsafe//:markupsafe"],
 )""",
             )
-
         # Json.
         if not native.existing_rule("json"):
             http_archive(
@@ -365,20 +363,19 @@ py_library(
             )

         if not native.existing_rule("markupsafe"):
-            http_archive(
-                name = "markupsafe",
-                urls = [
-                    "https://github.com/pallets/markupsafe/archive/1.0.tar.gz",
-                ],
-                sha256 = "dc3938045d9407a73cf9fdd709e2b1defd0588d50ffc85eb0786c095ec846f15",
-                strip_prefix = "markupsafe-1.0/markupsafe",
-                build_file_content = """py_library(
+                http_archive(
+                    name = "markupsafe",
+                    urls = [
+                        "https://github.com/pallets/markupsafe/archive/2.1.1.tar.gz",
+                    ],
+                    sha256 = "0f83b6d1bf6fa65546221d42715034e7e654845583a84906c5936590f9a7ad8f",
+                    strip_prefix = "markupsafe-2.1.1/src/markupsafe",
+                    build_file_content = """py_library(
     name = "markupsafe",
     visibility = ["//visibility:public"],
     srcs = glob(["*.py"])
 )""",
             )
-
     if analyzer_deps:
         if not native.existing_rule("google_bazel_common"):
             http_archive(
diff --git a/zetasql/base/arena_allocator.h b/zetasql/base/arena_allocator.h
index 4ffe711..d332343 100644
--- a/zetasql/base/arena_allocator.h
+++ b/zetasql/base/arena_allocator.h
@@ -109,7 +109,7 @@ template <class T, class C> class ArenaAllocator {
   ArenaAllocator(C* arena) : arena_(arena) { }  // NOLINT

   pointer allocate(size_type n,
-                   std::allocator<void>::const_pointer /*hint*/ = nullptr) {
+                   typename std::allocator_traits<std::allocator<void>>::const_pointer name = nullptr) {
     assert(arena_ && "No arena to allocate from!");
     return reinterpret_cast<T*>(arena_->AllocAligned(n * sizeof(T),
                                                      kAlignment));
diff --git a/zetasql/resolved_ast/BUILD b/zetasql/resolved_ast/BUILD
index 6839921..d85d73d 100644
--- a/zetasql/resolved_ast/BUILD
+++ b/zetasql/resolved_ast/BUILD
@@ -24,6 +24,7 @@ py_binary(
     srcs = [
         "gen_resolved_ast.py",
     ],
+    python_version = "PY3",
     deps = [
         ":resolved_ast_enums_py_pb2",
         "@io_abseil_py//absl:app",

Question

How can I resolve the issue with this generated protobuf source? Its not super clear to me why the issue is happening. I would really appreciate some guidance on how I could adjust my patch to resolve the problems with this generator.

Thanks!