-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat(symbolize): Symbolize detection support HAVE_LINK_H #1114
feat(symbolize): Symbolize detection support HAVE_LINK_H #1114
Conversation
link.h is usually provided by glibc. Current implementation of symbolize can work on link.h. Enhance the symbolize detection logic to support link.h, AKA. define HAVE_SYMBOLIZE if defined(HAVE_LINK_H).
Thanks for the PR. However, my comment from #1085 still stands as I do not want to rely on assumptions but rather use the build system to detect (or specify, in Bazel case) the feature availability. This also allows to keep the preprocessor logic (which is not straightforward to debug) simple. |
@sergiud I understand your opinion. To further discuss the difference in our views, we need to define the boundaries of the problem, i.e., what kind of functionality should be detected and provided by the build system, and what kind of functionality should be detected by the macro system. I believe my change is brief and simple to understand. We have detected I would like you to provide further information to help me understand on what principles we should base our decision on this issue. |
I provided a very clear explanation why the detection should be performed by the build system and not by the preprocessor. What information are you missing exactly? Also, I do not understand why do you keep pushing for a preprocessor based solution? Why can't you update the Bazel files instead? |
@sergiud I feel you are annoyed about the discussion. I agree we shouldn't waste too much time on quite "simple" problem. However, at least 2 warm community contributors are confused in this problem, so I think it's not trivial. I totally understand you prefer to solve the problem by build system rather than by the preprocessor. And I believe I understand your justification. (But I don't think re-implement the detection logic in each build systems with each build system specific DSL are even better than implement it in the preprocessor.) What confused me are
Now, I want to enhance the detection logic, where should be the correct place? What kind of functionality should be detected and provided by the build system, and what kind of functionality should be detected by the macro system? These questions are exactly what I was asking for. If the rule is we stop adding any logic to the preprocessor and gradually move all (this detection) logic from the preprocessor to the build system, I would happily follow it. I mean I never aware of that your are announcing a new rule, are you? |
You keep ignoring my reasoning for implementing the logic using the build system instead of the preprocessor which is why the discussion is not moving forward. To add to my previous comments:
I already answered this question here. Please limit the modifications to Bazel files only.
This question is irrelevant for solving this specific problem. In general, however, there must be a good reason for using the preprocessor instead of the build system. |
Thanks for your clarification. You have made a design decision but nobody in the community know about it. You said you prefer to put the detection logic in build system but the community didn't know it had been settled without any room for a further discussion. I would rather you just say that we have made a rule, just follow the rule to keep consistency. I would submit a new PR with the 2 changes:
Let me know if you have any concerns. |
Given this is your first contribution to glog, I don't believe you are in the position to speak on behalf of the community.
You seem to have an attitude problem. I don't appreciate neither your baseless accusations nor your attempts of twisting of my words. I'm asking you to keep the discussions civilized and limited to technical problems only, refraining from any personal attacks. Your behavior will not be tolerated. You have been warned. |
@sergiud , I apologize if my previous messages came across as confrontational. It was never my intention to offend or misrepresent your viewpoints. I recognize there may have been some gaps in our communication, and there are aspects of the project’s background and the discussions that I am not fully aware of, which may have led to a lack of common understanding. I truly value a harmonious work environment and am committed to improving how I communicate, ensuring it's based on well-substantiated facts rather than assumptions. I appreciate your patience and guidance as we work through this issue, and I am keen to adapt my approach to align better with the project’s methodologies and expectations. Thank you for addressing these concerns directly, and I look forward to making a constructive contribution moving forward. |
Thank you. Let's move forward with #1116. |
link.h
is usually provided by glibc. Current implementation of symbolize can work onlink.h
. Enhance the symbolize detection logic to supportlink.h
, AKA. defineHAVE_SYMBOLIZE
ifdefined(HAVE_LINK_H)
.This PR close #1084 and supersede #1085.
There's a few discussion in #1085 about why make changes in
src/symbolize.h
instead ofbazel/glog.bzl
. My justification is that this is an enhancement to symbolize detection logic rather than an alignment between bazel and cmake build systems.link.h
can provide usElfW
thus can support our implementation.I would like to open a separate issue if we still want to discuss whether we should define
HAVE_SYMBOLIZE
inbazel/glog.bzl
.