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

Permit __DATE__, __TIME__, etc in foreign builds #239

Closed
rnburn opened this issue Mar 29, 2019 · 22 comments
Closed

Permit __DATE__, __TIME__, etc in foreign builds #239

rnburn opened this issue Mar 29, 2019 · 22 comments

Comments

@rnburn
Copy link

rnburn commented Mar 29, 2019

I'm using configure_make with apache and get this error

<command-line>:0:10: error: 'redacted' undeclared here (not in a function); did you mean 'rename'?
/root/.cache/bazel/_bazel_root/f8087e59fd95af1ae29e8fcb7ff1a3dc/sandbox/linux-sandbox/5/execroot/apache_bazel_example/external/org_apache_httpd/server/buildmark.c:21:36: note: in expansion of macro '__DATE__'
 static const char server_built[] = __DATE__ " " __TIME__;

because of the options

-Wno-builtin-macro-redefined -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted"

Could those be disabled by default for these external builds?

@jwnimmer-tri
Copy link

From the peanut gallery (not a maintainer): the problem seems to be that the double-quoting is being dropped. A literal "redacted" " " "redacted" would have been just fine. Shell escaping must be wrong somewhere?

@UlysseM
Copy link

UlysseM commented Mar 29, 2019

I doubt you'd ever want to allow these, as they would change build artifacts all the time, which prevent any kinds of caching that bazel relies on.

Try adding this:
configure_options = ["CFLAGS='-Dredacted=\"redacted\"'"],

@rnburn
Copy link
Author

rnburn commented Mar 29, 2019

Yes, it seems as if the problem is that they're not escaped correctly.

I don't know that it's important for apache - but if they're going to be fixed, I would suggest at least setting them to valid expansions in case there's code that parses them.

@tonybase
Copy link

tonybase commented Jun 26, 2019

I'm using cmake_external to compile rocksdb and get this error

<command-line>: error: 'redacted' was not declared in this scope
<command-line>: note: in expansion of macro 'redacted'
note: in expansion of macro '__DATE__'

I don't found configure_options attrs from docs.
Could those be disabled by default for these external builds?

@adam-rocska
Copy link

adam-rocska commented Sep 16, 2019

I ran into the very same error trying to compile php (yeah...) :
In case someone would like to reproduce it : https://github.com/adam-rocska/rules_php

/Users/rocskaadam/bin/bazel build --tool_tag=ijwb:IDEA:ultimate --curses=no --color=yes --progress_in_terminal_title=no -- //php:php

And in my local user .bazelrc :

common --javabase @bazel_tools//tools/jdk:absolute_javabase --define=ABSOLUTE_JAVABASE=/Users/rocskaadam/lib/jdk-12.0.2.jdk --sandbox_debug --verbose_failures 

test //... --test_output=errors

Update 1 :

It's still compiling, but @UlysseM 's fix does seem to do the trick :
configure_options = ["CFLAGS='-Dredacted=\"redacted\"'"],

Update 2 :

Yes! It did help. Now I'll just have to dig through the rest of my problems.

Workaround that worked for me

@UlysseM 's fix : configure_options = ["CFLAGS='-Dredacted=\"redacted\"'"],

@guoqi
Copy link

guoqi commented Dec 16, 2019

If i really need this macro, what could i do to disable bazel's replacement? I could bear some loss of performance, but i don't find some solutions so far.

@PalaceChan
Copy link

How can this replacement of those macros be disabled? There are use cases which can bear the performance hit but require this

irfansharif added a commit to cockroachdb/rocksdb that referenced this issue Oct 17, 2020
Due to bazelbuild/rules_foreign_cc#239, we
elide usage of __DATE__ in RocksDB. It makes our life much easier when
trying to bazel-ify the cockroachdb repo.
@ilanbiala
Copy link

I'm also using cmake_external() and hitting this issue. Is there a workaround/fix when using this rule?

@itdaniher
Copy link

@ilanbiala after a bemusing round of string-escape tetris, I have found a solution!

env_vars = {"CFLAGS": "-Dredacted='\\\"redacted\\\"'"},

The same would hold for CXXFLAGS.

@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!

@attilaolah
Copy link
Contributor

The real problem here appears to be that the quoting seems to be different in the CMake and Configure/Make versions.

Here is how build_script.sh looks for a configure_make project (Python in this example):

ARFLAGS="rcsD" ASFLAGS="-U_FORTIFY_SOURCE -Wall -Wthread-safety -Wself-assign -fcolor-diagnostics -fno-omit-frame-pointer -fstack-protector -D__DATE__=\"redacted\" -D__TIME__=\"redacted\" -D__TIMESTAMP__=\"redacted\" -Wno-builtin-macro-redefined -no-canonical-prefixes" CFLAGS="-U_FORTIFY_SOURCE -Wall -Wthread-safety -Wself-assign -fcolor-diagnostics -fno-omit-frame-pointer -fstack-protector -D__DATE__=\"redacted\" -D__TIME__=\"redacted\" -D__TIMESTAMP__=\"redacted\" -Wno-builtin-macro-redefined -no-canonical-prefixes -DDATE='\"redacted\"' -DTIME='\"redacted\"'" CXXFLAGS="-U_FORTIFY_SOURCE -Wall -Wthread-safety -Wself-assign -fcolor-diagnostics -fno-omit-frame-pointer -fstack-protector -std=c++17 -D__DATE__=\"redacted\" -D__TIME__=\"redacted\" -D__TIMESTAMP__=\"redacted\" -Wno-builtin-macro-redefined -no-canonical-prefixes" LDFLAGS="-B${EXT_BUILD_ROOT}/external/llvm/bin --ld-path=${EXT_BUILD_ROOT}/external/llvm/bin/ld.lld -Wl,-no-as-needed -Wl,-z,relro,-z,now -lstdc++ -lm -no-canonical-prefixes -L$EXT_BUILD_DEPS/ffi_lib/lib -L$EXT_BUILD_DEPS/mpdecimal_lib/lib -L$EXT_BUILD_DEPS/z_lib/lib -L$EXT_BUILD_DEPS/openssl_lib/lib" AR="/.${EXT_BUILD_ROOT}/external/llvm/bin/llvm-ar" CC="/.${EXT_BUILD_ROOT}/external/llvm/bin/clang" CXX="/.${EXT_BUILD_ROOT}/external/llvm/bin/clang" CPPFLAGS="-I$EXT_BUILD_DEPS/ffi_lib/include -I$EXT_BUILD_DEPS/mpdecimal_lib/include -I$EXT_BUILD_DEPS/z_lib/include -I$EXT_BUILD_DEPS/openssl_lib/include" "$EXT_BUILD_ROOT/external/lib_python/configure" --prefix=$BUILD_TMPDIR/$INSTALL_PREFIX --disable-shared --with-openssl="${EXT_BUILD_DEPS}/openssl_lib" --with-system-libmpdec

While cmake() puts these into a crosstool_bazel.cmake script, which has a crazy number of escaping going on:

cat > crosstool_bazel.cmake << EOF
set(CMAKE_AR "/.${EXT_BUILD_ROOT}/external/llvm/bin/llvm-ar" CACHE FILEPATH "Archiver")
set(CMAKE_ASM_FLAGS_INIT "-U_FORTIFY_SOURCE -Wall -Wthread-safety -Wself-assign -fcolor-diagnostics -fno-omit-frame-pointer -fstack-protector -D__DATE__='\\"\\\\\\"redacted\\"\\\\\\"' -D__TIME__='\\"\\\\\\"redacted\\"\\\\\\"' -D__TIMESTAMP__='\\"\\\\\\"redacted\\"\\\\\\"' -Wno-builtin-macro-redefined -no-canonical-prefixes")
set(CMAKE_CXX_COMPILER "/.${EXT_BUILD_ROOT}/external/llvm/bin/clang")
set(CMAKE_CXX_FLAGS_INIT "-U_FORTIFY_SOURCE -Wall -Wthread-safety -Wself-assign -fcolor-diagnostics -fno-omit-frame-pointer -fstack-protector -std=c++17 -D__DATE__='\\"\\\\\\"redacted\\"\\\\\\"' -D__TIME__='\\"\\\\\\"redacted\\"\\\\\\"' -D__TIMESTAMP__='\\"\\\\\\"redacted\\"\\\\\\"' -Wno-builtin-macro-redefined -no-canonical-prefixes")
set(CMAKE_C_COMPILER "/.${EXT_BUILD_ROOT}/external/llvm/bin/clang")
set(CMAKE_C_FLAGS_INIT "-U_FORTIFY_SOURCE -Wall -Wthread-safety -Wself-assign -fcolor-diagnostics -fno-omit-frame-pointer -fstack-protector -D__DATE__='\\"\\\\\\"redacted\\"\\\\\\"' -D__TIME__='\\"\\\\\\"redacted\\"\\\\\\"' -D__TIMESTAMP__='\\"\\\\\\"redacted\\"\\\\\\"' -Wno-builtin-macro-redefined -no-canonical-prefixes")
set(CMAKE_EXE_LINKER_FLAGS_INIT "-B${EXT_BUILD_ROOT}/external/llvm/bin --ld-path=${EXT_BUILD_ROOT}/external/llvm/bin/ld.lld -Wl,-no-as-needed -Wl,-z,relro,-z,now -lstdc++ -lm -no-canonical-prefixes")
set(CMAKE_SHARED_LINKER_FLAGS_INIT "-shared -B${EXT_BUILD_ROOT}/external/llvm/bin --ld-path=${EXT_BUILD_ROOT}/external/llvm/bin/ld.lld -Wl,-no-as-needed -Wl,-z,relro,-z,now -lstdc++ -lm -no-canonical-prefixes")
EOF

Note the defines: -D__DATE__='\\"\\\\\\"redacted\\"\\\\\\"'. Something is clearly not right here, but I'm not sure where all these quotes are coming from.

@attilaolah
Copy link
Contributor

One workaround I found was to have r"-D__DATE__=\"redacted\"" in the crosstool config, but then for Python also pass in configure_env_vars = {"CFLAGS": r"""-DDATE='\"redacted\"' -DTIME='\"redacted\"'"""}. This way even though __DATE__ will not have the right escaping, Python will default to use DATE (and TIME) instead and just ignore __DATE__ and __TIME__).

This works until the quoting can be mode more consistent.


A good example that uses cmake() and the timestamp macros is Exiv2, e.g. here: https://github.com/attilaolah/wasm/blob/main/lib/exiv2/BUILD.bazel

@jsharpe
Copy link
Collaborator

jsharpe commented Aug 28, 2021

Hopefully the quoting is correct now for CMake based builds. The quotes -D__DATE__='\\"\\\\\\"redacted\\"\\\\\\"'. that @attilaolah observed was indeed incorrect.

Unfortunately for configure style builds there isn't one set of quoting that works everywhere 😭 This is because some configure scripts aren't autotools based scripts and actually are written in other languages e.g. perl or sh and these handle the number of required escapes differently.

For those asking about disabling this - these redactions are configured as part of the cc_toolchain; if you want to turn it off you need to define your own toolchain to do this. If you need help in doing so I'd suggest that you ask on the bazel-discuss mailing list or in the bazel slack.

@attilaolah
Copy link
Contributor

attilaolah commented Dec 5, 2021

For anyone reading the previous comment, here is a quick link to how one might enable/disable these in the unfiltered_compile_flags option:

https://github.com/attilaolah/wasm/blob/b45df456d0f8b1e4ef414282f2b7848b2b89b55b/toolchains/cc/clang.bzl#L129-L141

@github-actions
Copy link

github-actions bot commented Jun 3, 2022

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!

@jheaff1
Copy link
Collaborator

jheaff1 commented Jun 4, 2022

I submitted a PR (bazelbuild/bazel#14735) to be able to remove all default flags from the built-in cc toolchains.

I submitted it 4 months ago and it still hasn’t been reviewed yet 😔

@github-actions
Copy link

github-actions bot commented Dec 1, 2022

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!

@jvolkman
Copy link
Contributor

jvolkman commented Jan 8, 2023

@jheaff1 FYI looks like your upstream PR was merged.

@jheaff1
Copy link
Collaborator

jheaff1 commented Jan 8, 2023

Yep glad to see it’s in Bazel 6.0!

@jsharpe
Copy link
Collaborator

jsharpe commented Feb 28, 2023

Given bazelbuild/bazel#14735 is merged and there's not a lot we can do about the quoting situation as different build scripts require different styles of quoting, I'm going to close this now.

@jsharpe jsharpe closed this as completed Feb 28, 2023
@carlosgalvezp
Copy link

One problem of using 'configure_options' like this is that CFLAGS are being redefined, overriding the toolchain flags entirely. Are there less invasive ways to do it?

@attilaolah
Copy link
Contributor

Another library that seems to break due to the quoting here is gmp, which generates a gmp.h that expects no quotes in CFLAGS :(

#define __GMP_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""

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