Packaging update for 1.33.0 release #1433
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
-o is not available on macOS 12.7 at least, and it's what homebrew seems to support still. Also, the proposed switch seems to be used already in the codebase.
If we compile Unit with -Wstrict-overflow=5 (as we do with clang) then
we get the following warning
cc -c -pipe -fPIC -fvisibility=hidden -O0 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wstrict-overflow=5 -Wmissing-prototypes -g -I src -I build/include \
\
\
-o build/src/nxt_conf.o \
-MMD -MF build/src/nxt_conf.dep -MT build/src/nxt_conf.o \
src/nxt_conf.c
src/nxt_conf.c: In function ‘nxt_conf_json_parse_value’:
src/nxt_conf.c:1444:5: warning: assuming signed overflow does not occur when changing X +- C1 cmp C2 to X cmp C2 -+ C1 [-Wstrict-overflow]
1444 | if (nxt_fast_path((ch - '0') <= 9)) {
|
Does this actually cause an issue?... well, yes. Using this minimal test
config to show the problem
{
"listeners": {
"[::1]:8080": {
"pass": --100
}
}
}
With the above if () statement that triggers the warning, my assumption
here is that we only want a digit now. '0' - '9'.
ch is a u_char, however if ch is any character with an ASCII code < 48
('0') e.g if ch is '-' (45) then we get 45 - 48 = -3, through arithmetic
conversion, which makes the if () statement true (when it shouldn't) then
at some point we get the following error returned from the controller
{
"error": "Memory allocation failed."
}
Instead of the expected
{
"error": "Invalid JSON.",
"detail": "A valid JSON value is expected here. It must be either a literal (null, true, or false), a number, a string (in double quotes \"\"), an array (with brackets []), or an object (with braces {}).",
"location": {
"offset": 234,
"line": 15,
"column": 27
}
}
Casting the result of (ch - '0') to u_char resolves this issue, this
makes the above calculation come out as 253 (relying on unsigned integer
wraparound) which was probably the intended way for it to work.
Reviewed-by: Zhidao Hong <[email protected]>
Signed-off-by: Andrew Clayton <[email protected]>
This is unused, yet a community member just spent time finding and fixing a bug in it only to be told it's unused. Just get rid of the thing. Link: <nginx#963> Reviewed-by: Zhidao Hong <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
p is not used again before returning from the function. Found by the clang static analyser. Reviewed-by: Zhidao Hong <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
In nxt_router_temp_conf() we have
rtcf = nxt_mp_zget(mp, sizeof(nxt_router_conf_t));
if (nxt_slow_path(rtcf == NULL)) {
goto fail;
}
If rtcf is NULL then we do
fail:
if (rtcf->tstr_state != NULL) {
nxt_tstr_state_release(rtcf->tstr_state);
}
In which case we will dereference the NULL pointer rtcf.
This patch re-works the goto labels to make them more specific to their
intended purpose and ensures we are freeing things which have been
allocated.
This was found by the clang static analyser.
Reviewed-by: Zhidao Hong <[email protected]>
Signed-off-by: Andrew Clayton <[email protected]>
To enable tests that require privileged root access, this commit tests with `sudo`. The Java and Python jobs have additional permissions issues, so they are also configured and made with `sudo`. A small permissions fix is required before running tests to allow non-root users to execute within the `/home/runner` directory. This change also removes the custom directories that were required without root access. Reviewed-by: Andrew Clayton <[email protected]> Signed-off-by: Dylan Arbour <[email protected]>
This adds a GitHub CI workflow for the new wasm-wasi-component language module. Some things of note. 1) We need to special case 'wasm-wasi-component' in the 'Output build metadata' section as we are splitting the module names on '-' to split them into name and version. 2) Apart from needing to tell bindgen about the njs include paths, we also need to explicitly specify which version of clang to use to work around an issue with multiple versions of clang installed. Link: <https://gitlab.freedesktop.org/mesa/mesa/-/issues/7268> Signed-off-by: Andrew Clayton <[email protected]>
I changed a setting and now GitHub will recognize both the legacy numberless version, and the newer version with UserID. The with-UserID version will be used by the any changes stemming from the GitHub GUI. Reviewed-by: Andrew Clayton <[email protected]> Signed-off-by: Dylan Arbour <[email protected]>
The variables accessed with JS template literal should not be cacheable.
Since it is parsed by njs engine, Unit can't create indexes on these
variables for caching purpose. For example:
{
"format": "`{bodyLength:\"${vars.body_bytes_sent}\",status:\"${vars.status}\"}\n`"
}
The variables like the above are not cacheable.
Closes: nginx#1169
We don't run on Windows and only really support compiling Unit with GCC and Clang. Cc: Dan Callahan <[email protected]> Co-developed-by: Alejandro Colomar <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
We only really support building Unit with GCC and Clang. Cc: Dan Callahan <[email protected]> Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
We really only support building Unit with GCC and Clang. Cc: Dan Callahan <[email protected]> Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
We really only support building Unit with GCC and Clang. Cc: Dan Callahan <[email protected]> Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
This is what -Wextra used to be called, but any version of GCC or Clang in at least the last decade has -Wextra. Cc: Dan Callahan <[email protected]> Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
Expand on the comment on why we don't enable -Wstrict-overflow=5 on GCC. Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96658> Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
Aliasing is essentially when you access the same memory via different types. If the compiler knows this doesn't happen it can make some optimisations. There is however code in Unit, for example in the wasm language module and the websocket code that may fall foul of strict-aliasing rules. (For the wasm module I explicitly disable it there) In auto/cc/test for GCC we have NXT_CFLAGS="$NXT_CFLAGS -O" ... # -O2 enables -fstrict-aliasing and -fstrict-overflow. #NXT_CFLAGS="$NXT_CFLAGS -O2" #NXT_CFLAGS="$NXT_CFLAGS -Wno-strict-aliasing" So with GCC by default we effectively compile with -fno-strict-aliasing. For clang we have this NXT_CFLAGS="$NXT_CFLAGS -O" ... #NXT_CFLAGS="$NXT_CFLAGS -O2" ... NXT_CFLAGS="$NXT_CFLAGS -fstrict-aliasing" (In _clang_, -fstrict-aliasing is always enabled by default) So in clang we always build with -fstrict-aliasing. I don't think this is the best idea, building with something as fundamental as this disabled in one compiler and enabled in another. This patch adjusts the Clang side of things to match that of GCC. I.e compile with -fno-strict-aliasing. It also explicitly sets -fno-strict-aliasing for GCC, which is what we were getting anyway but lets be explicit about it. Cc: Dan Callahan <[email protected]> Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
This causes signed integer & pointer overflow to have a defined behaviour of wrapping according to two's compliment. I.e INT_MAX will wrap to INT_MIN and vice versa. This is mainly to cover existing cases, not an invitation to add more. Cc: Dan Callahan <[email protected]> Suggested-by: Alejandro Colomar <[email protected]> Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
The idea is rather than printing out the full compiler/linker etc
command for each recipe e.g
cc -c -pipe -fPIC -fvisibility=hidden -O0 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wno-strict-aliasing -Wmissing-prototypes -g -I src -I build/include \
\
\
-o build/src/nxt_cgroup.o \
-MMD -MF build/src/nxt_cgroup.dep -MT build/src/nxt_cgroup.o \
src/nxt_cgroup.c
Print a clearer abbreviated message e.g the above becomes
CC build/src/nxt_cgroup.o
This vastly reduces the noise when compiling and most of the time you
don't need to see the full command being executed.
This also means that warnings etc show up much more clearly.
You can still get the old verbose output by passing V=1 to make e.g
$ make V=1 ...
NOTE: With recent versions of make(1) you can get this same, verbose,
behaviour by using the --debug=print option.
This introduces the following message types
CC Compiling a source file to an object file.
AR Producing a static library, .a archive file.
LD Producing a dynamic library, .so DSO, or executable.
VER Writing version information.
SED Running sed(1).
All in all this improves the developer experience.
Subsequent commits will make use of this in the core and modules.
NOTE: This requires GNU make for which we check. On OpenIndiana/illumos
we have to use gmake(1) (GNU make) anyway as the illumos make doesn't
work with our Makefile as it is. Also macOS seems to generally install
GNU make.
We could make it work (probably) on other variants of make, but the
complexity starts increasing exponentially.
In fact we still print the abbreviated messages in the verbose output so
you can still do
$ make | grep ^" [A-Z]"
on other makes to effectively get the same output.
Co-developed-by: Alejandro Colomar <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
Signed-off-by: Andrew Clayton <[email protected]>
This makes use of the infrastructure introduced in the previous commit to pretty print the make output when building the Unit core and the C test programs. When building Unit the output now looks like VER build/include/nxt_version.h (NXT_VERSION) VER build/include/nxt_version.h (NXT_VERNUM) CC build/src/nxt_lib.o CC build/src/nxt_gmtime.o ... CC build/src/nxt_cgroup.o AR build/lib/libnxt.a CC build/src/nxt_main.o LD build/sbin/unitd SED build/share/man/man8/unitd.8 I'm sure you'll agree that looks much nicer! You can still get the old verbose output with $ make V=1 ... Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
This makes use of the infrastructure introduced in a previous commit, to pretty print the make output when building the Java language module. You can still get the old verbose output with $ make V=1 ... Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
This makes use of the infrastructure introduced in a previous commit, to pretty print the make output when building the Perl language module. You can still get the old verbose output with $ make V=1 ... Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
This makes use of the infrastructure introduced in a previous commit, to pretty print the make output when building the PHP language module. You can still get the old verbose output with $ make V=1 ... Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
This makes use of the infrastructure introduced in a previous commit, to pretty print the make output when building the Python language module. You can still get the old verbose output with $ make V=1 ... Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
This makes use of the infrastructure introduced in a previous commit, to pretty print the make output when building the Ruby language module. You can still get the old verbose output with $ make V=1 ... Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
This makes use of the infrastructure introduced in a previous commit, to pretty print the make output when building the wasm language module. You can still get the old verbose output with $ make V=1 ... Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
One issue you have when trying to debug Unit under say GDB is that at the default optimisation level we use of -O (-O1) the compiler will often optimise things out which means they are not available for inspection in the debugger. This patch allows you to pass 'D=1' to make, e.g $ make D=1 ... Which will set -O0 overriding the previously set -O, basically disabling optimisations, we could use -Og, but the clang(1) man page says this is best and it seems to not cause any issues when debugging GCC generated code. Co-developed-by: Alejandro Colomar <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
Having -Werror enabled all the time when developing can be a nuisance, allow to disable it by passing E=0 to make, e.g $ make E=0 ... This will set -Wno-error overriding the previously set -Werror. Co-developed-by: Alejandro Colomar <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
This adds a help target to the Makefile in the repository root that
shows what variables are available to control the make/build behaviour.
It currently looks like
$ make help
Variables to control make/build behaviour:
make V=1 ... - Enables verbose output
make D=1 ... - Enables debug builds (-O0)
make E=0 ... - Disables -Werror
Variables can be combined.
Reviewed-by: Alejandro Colomar <[email protected]>
Signed-off-by: Andrew Clayton <[email protected]>
This variable is _appended_ to the main CFLAGS variable and allows setting extra compiler options at make time. E.g $ make EXTRA_CFLAGS="..." ... Useful for quickly testing various extra warning flags. Suggested-by: Alejandro Colomar <[email protected]> Reviewed-by: Alejandro Colomar <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
- Adds `unitctl/` prefix to tags generated by manual workflow runs. Previously, only release titles (but not tags) were prefixed. - Omits superfluous `name` field; falls back to `tag` when absent. - Removes unnecessary conditional from `prelease` field. This results in the following tagging / releasing behavior: 1. Running manually creates a pre-release and tags it `unitctl/VERSION` 2. Pushing a tag formatted like `x.y.z` creates a normal release Refines: 3501a50
Wasm module is now not built for Amazon Linux 2, Debian 11 and Ubuntu 2.0.04, since it requires cmake version newer than what's available on those OSes. wasm-wasi-component is not affected.
With commit 9998918 ("Packages: bump wasmtime to 24.0.0 and wasi-sysroot to 24.0.") the paths to the wasmtime C API include and lib directories changed which broke the wasm ci tests. Signed-off-by: Andrew Clayton <[email protected]>
This will catch changes to the likes of wasmtime and njs. Signed-off-by: Andrew Clayton <[email protected]>
Don't try and run the tests that require njs if it isn't enabled. Closes: nginx#1411 Fixes: 43c4bfd ("tests: "if" option in http route match") Signed-off-by: Andrew Clayton <[email protected]>
Suppress the output from cargo-component when we first run it to check if it's available, otherwise you may see the following $ pytest test/test_wasm-wasi-component.py which: no go in (/home/andrew/.local/bin:/home/andrew/bin:/usr/share/Modules/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin) error: no such command: `component` View all installed commands with `cargo --list` Find a package to install `component` with `cargo search cargo-component Note: This didn't stop the tests from working, just an aesthetic issue. Closes: nginx#1410 Signed-off-by: Andrew Clayton <[email protected]>
When the client closes the connection before the upstream, the proxy's error handler was calling cleanup operation like peer close and request close twice, this fix ensures the cleanup is performed only once, improving proxy stability. Closes: nginx#828
On some Python 3.11 systems, 3.11.9 & 3.11.10, we were seeing a crash
triggered by Py_Finalize() in nxt_python_atexit() when running one of
our pytests, namely
test/test_python_factory.py::test_python_factory_invalid_callable_value
2024/09/12 15:07:29 [alert] 5452#5452 factory "wsgi_invalid_callable" in module "wsgi" can not be called to fetch callable
Fatal Python error: none_dealloc: deallocating None: bug likely caused by a refcount error in a C extension
Python runtime state: finalizing (tstate=0x00007f560b88a718)
Current thread 0x00007f560bde7ad0 (most recent call first):
<no Python frame>
2024/09/12 15:07:29 [alert] 5451#5451 app process 5452 exited on signal 6 (core dumped)
This was due to
obj = PyDict_GetItemString(PyModule_GetDict(module), callable);
in nxt_python_set_target() which returns a *borrowed* reference, then
due to the test meaning this is a `None` object we `goto fail` and call
Py_DECREF(obj);
which then causes `Py_Finalize()` to blow up.
The simple fix is to just increment its reference count before the `goto
fail`.
Note: This problem only showed up under (the various versions of Python
we test on); 3.11.9 & 3.11.10. It doesn't show up under; 3.6, 3.7, 3.9,
3.10, 3.12
Cc: Konstantin Pavlov <[email protected]>
Closes: nginx#1413
Fixes: a9aa9e7 ("python: Support application factories")
Signed-off-by: Andrew Clayton <[email protected]>
The two files under unit-openapi/.openapi-generator/, FILES and VERSIONS are auto-generated. Signed-off-by: Andrew Clayton <[email protected]>
Signed-off-by: Gabor Javorszky <[email protected]>
Signed-off-by: Gabor Javorszky <[email protected]>
This comes after internal conversation with the NGINX security council. Signed-off-by: Gabor Javorszky <[email protected]>
All new NGINX projects are created from the template repository which has a SECURITY.md file in it. This adopts the file. NOTE; We wrap the file around the 76-78 character mark for consistency and readability. Link: <https://github.com/nginxinc/template-repository> Closes: nginx#1408 Signed-off-by: Gabor Javorszky <[email protected]> [ Tweaked commit message - Andrew ] Signed-off-by: Andrew Clayton <[email protected]>
The correct capitalisation of the name of the software is Unit, not all caps. Signed-off-by: Gabor Javorszky <[email protected]> [ A bunch more s/UNIT/Unit/ - Andrew ] Signed-off-by: Andrew Clayton <[email protected]>
Signed-off-by: Gabor Javorszky <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
CONTROL_SOCKET_ADDRESS is singular, adds note that the flag can be specified multiple times, and adjusts code to print CONTROL_SOCKET_ADDRESS as singular. Signed-off-by: Gabor Javorszky <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
Signed-off-by: Ava Hahn <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
Signed-off-by: Andrew Clayton <[email protected]>
Signed-off-by: Andrew Clayton <[email protected]>
Signed-off-by: Andrew Clayton <[email protected]>
This is autogenerated from docs/changes.xml by $ make -C docs/ changes && mv build/CHANGES . Signed-off-by: Andrew Clayton <[email protected]>
Unit 1.33.0 release.
|
Merges cleanly into packaging locally... |
thresheek
force-pushed
the
packaging
branch
from
f38ed1f
to
4ca64ff
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Packaging update for the release.
No notable changes aside from the version bump