-
Notifications
You must be signed in to change notification settings - Fork 892
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
Some 32-bit archs require explicit liking to libatomic
, otherwise build fails: Undefined symbols: "___atomic_compare_exchange_8", "___atomic_fetch_xor_8" etc.
#321
Comments
@bellard I give up on this, makefile is designed in such a way that whichever attempts to link to the needed libs fail. We need a) It does not here.
I tried various ways, everything fails here. Could you help? |
Use the
|
@andrjohns I am sure I tried that, but I will try again and share a log if it fails. |
@andrjohns It still fails. I tried to keep changes to the minimum, so only fixed the compiler choice, since makefile forces clang or no-name gcc (which picks the wrong one), and this build had LTO enabled (which is useless on
Flags are passed, linking does not work. My earlier attempts were without LTO and flags added via patching makefile – but anyway, result is the same, neither works. |
Don't run |
In case it helps you may want to give the friendly fork a try: https://github.com/quickjs-ng/quickjs we switched the buildsystem to CMake, which should take care of this problem automagically. |
@saghul CMake should work, yeah. There was can even add a proper test for |
@saghul Builds nicely with CMake, the only minor error being:
So passing |
It does not seem to have a proper test target, but running manually it seems to do something :) Not sure what to make of this table:
|
There is a Makefile with a |
What compiler flags are you using? Thar looks like a false positive. Here is the offending function:
|
@saghul Maybe header for I did not add any special flags. |
Should be there. Can you please the compiler version and the verbose build output please? |
@barracuda156 you had a similar warning in your earlier logs:
It looks like while your compiler is in |
@andrjohns Thank you, I will check that. It should certainly not happen, but if CMakeLists override flags which MacPorts pass, that will cause trouble (and perhaps should be fixed generally, it is a wrong behavior). |
@saghul What are the plans by the way, merge two forks back or keep separate? Since the original repo is still maintained, I cannot perhaps switch the port to another one. If the projects are to be considered co-existing, I can make a new port and declare a conflict, so a user can choose which version he prefers. |
The output you posted is disturbing. What machine did ou use? I get much faster timings on macOS with my Macbook Air, even running on battery:
|
@barracuda156: The plan is to merge both forks sometime later this year. I am maintaining the original repository and also contribute to the ng fork. I have been slow to backport many patches because of business related duties, but I have set aside most of July and August to catch up on this goal. |
@chqrlie Thank you for the explanation. This is a 2005 PowerMac :) If something can be improved, I am eager to run builds and tests. |
@chqrlie Then perhaps it is not worth introducing a separate port in MacPorts just to delete it in a few months. However, the current makefiles build does not work for me. If CMake support can be added, I can try switching it to CMake in MacPorts, that will probably be fine. Otherwise if you can tell me how to fix linking, I can stay with makefiles build. (To be clear, this is not a typical issue, and perhaps quite unique: I have no idea why linking fails when flags are passed.) |
You can see in the logs you posted that the linking fails at this step:
Which is the makefile target example for compiling to an executable. This fails because The simplest workaround is to not run |
@andrjohns Regarding the compile error above: it happens with Release build, but not with Debug (everything else being the same).
With release, however:
Since |
That will be a questionable reason to justify a PR disabling a target: I cannot fix it the linking for it :) Examples are often skipped, not a big deal as such, but still it makes a better sense actually fix the linking. Looks like I can just patch the file which you have referred? |
Completely untested, but you could try something like the below to add the *arg++ = "-lm";
*arg++ = "-ldl";
*arg++ = "-lpthread";
+ const char* EXTRA_LIBS_ENV = getenv("EXTRA_LIBS");
+ if (EXTRA_LIBS_ENV) {
+ *arg++ = EXTRA_LIBS_ENV;
+ }
*arg = NULL; |
I would prefer a solution where |
This would be preferable indeed. |
After much tinkering with libuv's CI, I finally figured out that ASLR is the root cause for the ASan and MSan failures. Newer kernels use bigger PIE slides and the sanitizer runtimes don't know how to handle those (yet - looks like it's been fixed upstream.) Refs: quickjs-ng/quickjs#315 Refs: libuv/libuv#4365
This is seemingly problematic for us (NGINX packaging, we bundle quickjs as an engine for our JS VM implementation) as we'd really like to use I guess I can also not use |
The text was updated successfully, but these errors were encountered: