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

Compiling postgres with unquoted CONFIGURE_ARGS #442

Closed
jschaf opened this issue Dec 1, 2020 · 13 comments
Closed

Compiling postgres with unquoted CONFIGURE_ARGS #442

jschaf opened this issue Dec 1, 2020 · 13 comments

Comments

@jschaf
Copy link

jschaf commented Dec 1, 2020

I'm having trouble compiling Postgres, specifically with configure strings and C macros.

# WORKSPACE
all_content = """filegroup(name = "all_files", srcs = glob(["**"]), visibility = ["//visibility:public"])"""

http_archive(
    name = "postgres",
    build_file_content = all_content,
    strip_prefix = "postgresql-13.1",
    urls = [
        "https://ftp.postgresql.org/pub/source/v13.1/postgresql-13.1.tar.bz2",
    ],
    sha256 = "12345c83b89aa29808568977f5200d6da00f88a035517f925293355432ffe61f",
)
# BUILD
load("@rules_foreign_cc//tools/build_defs:configure.bzl", "configure_make")

configure_make(
    name = "postgres",
    lib_source = "@postgres//:all_files",
)

Stack trace

/usr/bin/gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla \
    -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wformat-security \
    -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation \
    -Wno-stringop-truncation -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter \
    -Wno-free-nonheap-object -fno-omit-frame-pointer -fno-canonical-system-headers \
    -Wno-builtin-macro-redefined -D__DATE__="redacted" -D__TIMESTAMP__="redacted" \
    -D__TIME__="redacted" -DFRONTEND -I. -I/home/joe/.cache/bazel/_bazel_joe/171db9775560bb7c0e50c299ed4d4528/sandbox/linux-sandbox/5/execroot/scrap/external/postgres/src/common \
    -I../../src/include \
    -I/home/joe/.cache/bazel/_bazel_joe/171db9775560bb7c0e50c299ed4d4528/sandbox/linux-sandbox/5/execroot/scrap/external/postgres/src/include  \
    -D_GNU_SOURCE  -DVAL_CC="\"/usr/bin/gcc\"" -DVAL_CPPFLAGS="\"-D_GNU_SOURCE\"" -DVAL_CFLAGS="\"-Wall \
    -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels \
    -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wformat-security -fno-strict-aliasing \
    -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation \
    -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object \
    -fno-omit-frame-pointer -fno-canonical-system-headers -Wno-builtin-macro-redefined \
    -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted"\"" \
    -DVAL_CFLAGS_SL="\"-fPIC\"" -DVAL_LDFLAGS="\"-fuse-ld=gold -Wl,-no-as-needed \
    -Wl,-z,relro,-z,now -B/usr/bin -pass-exit-codes -lstdc++ -lm -Wl,--as-needed \
    -Wl,-rpath,'/tmp/tmp.PYKGNrVMhs/postgres/lib',--enable-new-dtags\"" -DVAL_LDFLAGS_EX="\"\"" \
    -DVAL_LDFLAGS_SL="\"\"" -DVAL_LIBS="\"-lpgcommon -lpgport -lpthread -lz -lreadline -lrt -ldl \
    -lm \""  -c -o config_info.o \
    /home/joe/.cache/bazel/_bazel_joe/171db9775560bb7c0e50c299ed4d4528/sandbox/linux-sandbox/5/execroot/scrap/external/postgres/src/common/config_info.c

In file included from /home/joe/.cache/bazel/_bazel_joe/171db9775560bb7c0e50c299ed4d4528/sandbox/linux-sandbox/5/execroot/scrap/external/postgres/src/include/c.h:54,
                 from /home/joe/.cache/bazel/_bazel_joe/171db9775560bb7c0e50c299ed4d4528/sandbox/linux-sandbox/5/execroot/scrap/external/postgres/src/include/postgres_fe.h:25,
                 from /home/joe/.cache/bazel/_bazel_joe/171db9775560bb7c0e50c299ed4d4528/sandbox/linux-sandbox/5/execroot/scrap/external/postgres/src/common/config_info.c:20:

/home/joe/.cache/bazel/_bazel_joe/171db9775560bb7c0e50c299ed4d4528/sandbox/linux-sandbox/5/execroot/scrap/external/postgres/src/common/config_info.c:
    In function 'get_configdata': ../../src/include/pg_config.h:47:281: error: expected ')' before 'redacted'

   47 | #define CONFIGURE_ARGS " '--prefix=/tmp/tmp.PYKGNrVMhs/postgres' 'CC=/usr/bin/gcc' 'CFLAGS=-U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -fno-canonical-system-headers -Wno-builtin-macro-redefined -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted"' 'LDFLAGS=-fuse-ld=gold -Wl,-no-as-needed -Wl,-z,relro,-z,now -B/usr/bin -pass-exit-codes -lstdc++ -lm' 'CPPFLAGS=' 'CXX=/usr/bin/gcc' 'CXXFLAGS=-U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -std=c++0x -fno-canonical-system-headers -Wno-builtin-macro-redefined -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted"'"
      |                                                                                                                                                                                                                                                                                         ^~~~~~~~

/home/joe/.cache/bazel/_bazel_joe/171db9775560bb7c0e50c299ed4d4528/sandbox/linux-sandbox/5/execroot/scrap/external/postgres/src/common/config_info.c:127:34: note: in expansion of macro 'CONFIGURE_ARGS'
  127 |  configdata[i].setting = pstrdup(CONFIGURE_ARGS);

The root of the problem seems to be the lack of string escaping in CONFIGURE_ARGS:

#define CONFIGURE_ARGS " '--prefix=/tmp/tmp.PYKGNrVMhs/postgres' 'CC=/usr/bin/gcc' 'CFLAGS=-U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -fno-canonical-system-headers -Wno-builtin-macro-redefined -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted"' 'LDFLAGS=-fuse-ld=gold -Wl,-no-as-needed -Wl,-z,relro,-z,now -B/usr/bin -pass-exit-codes -lstdc++ -lm' 'CPPFLAGS=' 'CXX=/usr/bin/gcc' 'CXXFLAGS=-U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -std=c++0x -fno-canonical-system-headers -Wno-builtin-macro-redefined -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted"'"

CONFIGURE_ARGS is created in the configure script with:

AC_DEFINE_UNQUOTED(CONFIGURE_ARGS, ["$ac_configure_args"], [Saved arguments from configure])

I've tried the recommendation from #239 but it also errors.

    configure_options = ["CFLAGS='-Dredacted=\"redacted\"'"],

The resulting macro definition is:

../../src/include/pg_config.h:47:86: note: in expansion of macro 'redacted'
   47 | #define CONFIGURE_ARGS " '--prefix=/tmp/tmp.tEUPVh8YFX/postgres' 'CFLAGS=-Dredacted="redacted"' 'CC=/usr/bin/gcc' 'LDFLAGS=-fuse-ld=gold -Wl,-no-as-needed -Wl,-z,relro,-z,now -B/usr/bin -pass-exit-codes -lstdc++ -lm' 'CPPFLAGS=' 'CXX=/usr/bin/gcc' 'CXXFLAGS=-U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -std=c++0x -fno-canonical-system-headers -Wno-builtin-macro-redefined -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted"'"

@jschaf
Copy link
Author

jschaf commented Dec 1, 2020

I think I have a workaround. I redefined CONFIGURE_ARGS in the configure_script using http_archive.patch_cmds:

        patch_cmds = [
            """sed -i 's/^#define CONFIGURE_ARGS "$ac_configure_args"/#define CONFIGURE_ARGS "redacted"/' configure""",
        ],

@rahulmutt
Copy link

rahulmutt commented Feb 27, 2021

Unfortunately, that override removes all configure flags which didn't work for my use case that requires building postgres with OpenSSL. The following worked for me and preserved the flags I needed to send to configure.

    configure_options = [
        "CFLAGS='-Dredacted=\\\"redacted\\\"'",
    ],

Likely after this PR is merged, this issue will disappear:

#362

@jschaf
Copy link
Author

jschaf commented Mar 1, 2021

Nice! I just ran into the need to have SSL support for Postgres. Would you mind sharing the configure_make invocation you're using?

@UebelAndre
Copy link
Collaborator

Likely after this PR is merged, this issue will disappear:

#362

If anyone had the bandwidth to put together another PR with a small example exercising the new code, I'd be happy to approve it. It seems like #362 has become a bit stale.

@rahulmutt
Copy link

rahulmutt commented Mar 2, 2021

# OpenSSL build rules
# https://github.com/bazelbuild/rules_foreign_cc/issues/337#issuecomment-657004174
CONFIGURE_OPTIONS = [
    "no-weak-ssl-ciphers",
    "no-idea",
    "no-comp",
]

configure_make(
    name = "openssl",
    configure_command = "config",
    configure_env_vars = select({
        "@platforms//os:macos": {
            "AR": "",
        },
        "//conditions:default": {},
    }),
    configure_options = select({
        "@platforms//os:macos": [
            "shared",
            "no-afalgeng",
            "ARFLAGS=r",
        ] + CONFIGURE_OPTIONS,i
        "//conditions:default": [
        ] + CONFIGURE_OPTIONS,
    }),
    lib_source = "@openssl//:all",
    shared_libraries = select({
        "@platforms//os:macos": [
            "libssl.dylib",
            "libcrypto.dylib",
        ],
        "//conditions:default": [
            "libssl.so",
            "libcrypto.so",
        ],
    }),
)

configure_make(
    name = "postgres-all",
    lib_source = "@postgres//:all",
    configure_env_vars = select({
        "@platforms//os:macos": {"AR": ""},
        "//conditions:default": {},
    }),
    configure_options = [
        "--with-openssl",
        "--with-libraries=$EXT_BUILD_DEPS/openssl/lib",
        "--with-includes=$EXT_BUILD_DEPS/openssl/include",
          # See: https://github.com/bazelbuild/rules_foreign_cc/pull/362
        "CFLAGS='-Dredacted=\\\"redacted\\\"'",
    ],
    binaries = [
        "pg_ctl",
        "createdb",
        "initdb",
        "psql",
        "pg_config",
        "postgres",
    ],
    deps = [
        ":openssl",
    ],
)

WORKSPACE

    http_archive(
        name = "openssl",
        build_file_content = all_content,
        strip_prefix = "openssl-OpenSSL_1_1_1j",
        url = "https://github.com/openssl/openssl/archive/OpenSSL_1_1_1j.tar.gz",
        sha256 = "22d6588e4a7c5ad48fcac2fbf1d035bd43258c22a49457dad0539ded0651b4d2"
    )

    http_archive(
       name = "postgres",
       build_file_content = all_content,
       strip_prefix = "postgresql-13.2",
       url = "https://ftp.postgresql.org/pub/source/v13.2/postgresql-13.2.tar.gz",
       sha256 = "3386a40736332aceb055c7c9012ecc665188536d874d967fcc5a33e7992f8080",
    )

@UebelAndre
Copy link
Collaborator

With the same justification as #537 (comment), can this be closed?

@jschaf
Copy link
Author

jschaf commented Mar 2, 2021

I don't think this is the same as #537 (that was more a documentation bug or PEBKAC). This bug deals with configure_make failing to handle double quoting in configure args. You could probably close it once #362 lands.

@aapeliv
Copy link

aapeliv commented Mar 2, 2021

Thank you for posting these postgres rules @jschaf and @rahulmutt, this is super useful!

Did either of you run into a configure error of configure: error: cannot detect from compiler exit status or warnings on macOS? This works well on Ubuntu 18.04 but having issues on my Mac (works also when doing ./configure outside bazel). (Modifying to CFLAGS='-Dredacted=\\\"redacted\\\" -fno-builtin' fixed this.)

@rahulmutt
Copy link

rahulmutt commented Mar 3, 2021

@aapeliv Out of curiosity which mac os version and which clang version did you have? I've also been using those rules on Mac and didn't need the extra -fno-builtin.

@UebelAndre
Copy link
Collaborator

Would #567 fix this?

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days. Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_foreign_cc!

@github-actions
Copy link

This issue was automatically closed because it went 30 days without a reply since it was labeled "Can Close?"

@kpark-hrp
Copy link

@aapeliv @rahulmutt Have either of you guys have been able to compile postgresql@14 on M1 Mac with rules_foreign_cc? I've been trying for days can't figure it out.

I keep getting

9: error: call to undeclared function 'sync_file_range'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
        (void) sync_file_range(fd, 0, 0, SYNC_FILE_RANGE_WRITE);
               ^
/private/var/tmp/_bazel_kevin.park/7b9ff2fa2a366cdd47b3c004eaf5f127/sandbox/darwin-sandbox/59/execroot/__main__/external/postgresql/src/common/file_utils.c:242:35: error: use of undeclared identifier 'SYNC_FILE_RANGE_WRITE'
        (void) sync_file_range(fd, 0, 0, SYNC_FILE_RANGE_WRITE);
                                         ^
2 errors generated.
make[2]: *** [<builtin>: file_utils.o] Error 1

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

No branches or pull requests

5 participants