Skip to content

Conversation

@TomJo2000
Copy link
Member

@TomJo2000 TomJo2000 requested a review from Grimler91 as a code owner September 8, 2025 14:46
@TomJo2000
Copy link
Member Author

Okay, what is going on here?
This builds just fine locally.

@TomJo2000 TomJo2000 force-pushed the bump-txikijs branch 2 times, most recently from 4c14832 to 4d24518 Compare September 8, 2025 15:00
@TomJo2000
Copy link
Member Author

Yeah it's cursed.
I don't know what's happening here.
It worked in the earlier PR and it works locally.

@robertkirkman
Copy link
Member

robertkirkman commented Sep 9, 2025

undeclared identifier LLONG_MAX

a similar error recently occurred in Nodejs that was fixed by thunder-coding:

Some clarification about thunder-coding's commit message: in Europe, the first official C specification is usually referred to as "C90", and in the United States, the first official C specification is usually referred to as "C89", and sometimes, through a confused game of telephone, people from each area consider only one of the names to be the correct name, or think that they are somehow "different", but people who know the full picture know the following:

  • both names are technically correct and actually refer to the same exact behavior definitions of the same exact programming language, and that is why -std=gnu89, -std=gnu90, -std=c89 and -std=c90 all work and are fully supported to the present day by most C compilers and platforms, where each set of "89/90" pairs are internally calculated as exactly the same language, a detail that is clearly visible in open source compilers:
  • any code that contains any uses of LLONG_MAX is technically not actually C89/C90 code because that definition is not part of the C official specification until C99 https://en.cppreference.com/w/c/types/limits.html.
    • Therefore, any build system attempting to compile any code that contains LLONG_MAX with any of the arguments containing "89" or "90" has a bug, in the build system code itself and not anywhere else (unless the original code is actually intended to remain as C89/C90 code, in which case the presence of LLONG_MAX there would be the bug)
    • The correct solution, correctly reached by thunder-coding, is that, in most cases, the code containing references to LLONG_MAX should be assumed to be intended to be C99+ code, not C89/C90 code, and its associated build systems should be edited to process it as explicitly -std=c99, -std=gnu99, -std=c11, -std=gnu11, or another C language standard that does officially support LLONG_MAX.

How Clang's official website documents their support for this confusingly-double-named C language variant:

https://clang.llvm.org/c_status.html

(scroll to the bottom)

"C89 implementation status"

"Clang implements all of the ISO 9899:1990 (C89) standard."

"You can use Clang in C89 mode with the -std=c89 or -std=c90 options."

image

@TomJo2000 I can reproduce the error locally, so you might want to try deleting all your docker containers and images and relaunching scripts/run-docker.sh to download a new, cleaned one.

Patch to place in packages/txikijs folder that fixes this error:

--- a/deps/libuv/CMakeLists.txt
+++ b/deps/libuv/CMakeLists.txt
@@ -20,7 +20,7 @@ include(CTest)
 set(CMAKE_C_VISIBILITY_PRESET hidden)
 set(CMAKE_C_STANDARD_REQUIRED ON)
 set(CMAKE_C_EXTENSIONS ON)
-set(CMAKE_C_STANDARD 90)
+set(CMAKE_C_STANDARD 11)
 
 set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
 

@TomJo2000
Copy link
Member Author

I found out why it builds locally.
There was a new patch introduced as part of #25895 that somehow didn't get added by git add when I made the commit for this branch.
That patch solves the issue in a slightly different way.

--- a/deps/libuv/src/unix/linux.c
+++ b/deps/libuv/src/unix/linux.c
@@ -26,6 +26,7 @@
 #include "uv.h"
 #include "internal.h"
 
+#define LLONG_MAX      9223372036854775807LL
 #include <inttypes.h>
 #include <stdatomic.h>
 #include <stddef.h>  /* offsetof */

I will be using Robert's patch as it seems like the more proper solution.

@robertkirkman
Copy link
Member

robertkirkman commented Sep 9, 2025

Agreed, my way is a better way, but the attempt of the other patch is credible as a successful messy workaround before a better solution was available.

Its (#define LLONG_MAX...) quality is comparable to the current workaround I explained for the error in the sequoia-sqv package in the Apt 3.0+ PR - a messy and not great workaround that is fine for testing or just getting the package to build locally, but which a better solution needs to be found to replace for a final version.

@TomJo2000
Copy link
Member Author

Oh did you actually come up with a fix for sequioa-sqv?
I hadn't had the time to get back to #24212 again yet.

Copy link
Member Author

@TomJo2000 TomJo2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would ya look at that, if you patch the thing that's causing the problem the package actually builds...

@robertkirkman
Copy link
Member

Oh did you actually come up with a fix for sequioa-sqv? I hadn't had the time to get back to #24212 again yet.

I have been able to force the package to build locally for me, but in a very messy way that is not appropriate for a final version yet. I explained how in my last comment there,

until me or someone else finds a better way, my workaround explained there can just be used as a temporary workaround for local building to try to get and test other packages that depend on sequoia-sqv, to work on other parts of the PR, for example.

@TomJo2000 TomJo2000 force-pushed the bump-txikijs branch 2 times, most recently from 1b3d602 to c4c76bc Compare September 9, 2025 11:52
Co-authored-by: xdrlmm <983751615@qq.com>
Co-authored-by: Robert Kirkman <rkirkman@termux.dev>
Copy link
Member

@robertkirkman robertkirkman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other patches removed look to me like they were written to fix build errors, so if those build errors do not happen anymore, they are probably unnecessary.

The one I don't know about is packages/txikijs/deps-libuv-src-unix-internal.h.patch, but the code that one patched no longer exists in the upstream so it is probably not necessary anymore.

@robertkirkman
Copy link
Member

packages/txikijs/deps-libuv-src-unix-internal.h.patch is confirmed no longer necessary in projects importing libuv after this commit:

libuv/libuv@06948c6

@TomJo2000 TomJo2000 merged commit c6908bc into termux:master Sep 9, 2025
11 checks passed
@TomJo2000 TomJo2000 deleted the bump-txikijs branch September 9, 2025 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants