nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.25k stars 322 forks source link

Merged with the 1.32 branch #1205

Closed andrey-zelenkov closed 1 week ago

ac000 commented 3 months ago

I'm afraid this is also wrong.

We will end up with the three bugfix commits that are already on master, duplicated on master again.

If you remember, we had this issue a while back with 1.29.0 and 1.29.1

Technically they are different commits, to git at least as they have different hashes. But they are the same commits and just pollute the history.

I see two options.

1) Manually update the changes etc

2) Do nothing and treat master and 1.32 as totally separate entities.

This would be a bit like the Linux kernel and the stable tree.

Linus' master branch only has the main releases 5.x, 6.x etc... The stable tree then does the point releases, 6.x.1, 6.x.2 etc

The stable branches only contain commits that are already on Linus' master.

So in our case master would simply go; 1.32 -> 1.33 -> 1.34 -> ...

andrey-zelenkov commented 3 months ago

If you remember, we had this issue a while back with 1.29.0 and 1.29.1

I remember this and I still think that crafting this commit manually is greater evil than a mere merge (since I don't see anything bad in duplicated commits).

Do nothing and treat master and 1.32 as totally separate entities.

This leave us with inconsistent CHANGES at least, that was reported by @tippexs and @Jcahilltorre

ac000 commented 3 months ago

I remember this and I still think that crafting this commit manually is greater evil than a mere merge
(since I don't see anything bad in duplicated commits).

Duplicate commits clutter and pollute the history reducing its effectiveness. They also skew repository statistics.

They could also raise some questions about "Which one do I revert?"

We would now have a history like

*   c36102a1 Merged with the 1.32 branch
|\  
| * 48d79170 Added version 1.32.1 CHANGES
| * 3f228d6b Generated Dockerfiles for Unit 1.32.1
| * 8ca4962d Edited changes.xml for the 1.32.1 release
| * 777b7c87 Wasm-wc: Fix application restarts
| * d7ff6bb4 Tests: NJS cacheable variables with access log
| * 6359c74d Var: Fix cacheable issue for njs variable access
| * 97ff0990 Version bump
* | e75f8d5d Wasm-wc: Fix application restarts
* | a8cfea8b Rebuild wasm-wasi-component when any of its dependencies change
* | b65e49c5 Build with -std=gnu11 (C11 with GNU extensions)
* | 7472a2ca Add a GitHub workflow status badge for our CI to the README
* | dc16a7bc Add a repostatus badge to the README
* | 0716b0c7 Tests: NJS cacheable variables with access log
* | 9993814d NJS: loader should be registered using njs_vm_set_module_loader()
* | abcfc4cd Fix the security-alert email link in the README
* | dd701fb4 Avoiding arithmetic ops with NULL pointer in nxt_unit_mmap_get
* | c9461a6b Initialize port_impl only when it is needed
* | 264b3755 Avoiding arithmetic ops with NULL pointer in nxt_port_mmap_get
* | 7dcd6c0e Avoiding arithmetic ops with NULL pointer in nxt_http_arguments_parse
* | 8844d33c Fixed undefined behaviour in left shift of int value
* | fdc46759 NJS: avoiding arithmetic ops with NULL pointer in r->args
* | 0d99744d Router: match when pattern and tested string are both zero length
* | 2e615250 Add dependabot.yml
* | 6b138571 Wasm-wc: Bump the mio crate from 0.8.10 to 0.8.11
* | a171b399 Add an EXTRA_CFLAGS make variable
* | f55fa70c Add a help target to the root Makefile
* | d23812b8 Allow to disable -Werror at 'make' time
* | 15072fbd Enable optional 'debuggable' builds
* | b763ba7e Pretty print the wasm language module compiler output
* | 133f75fd Pretty print the Ruby language module compiler output
* | caaa1d28 Pretty print the Python language module compiler output
* | 0a0dcf91 Pretty print the PHP language module compiler output
* | 574528f7 Pretty print the Perl language module compiler output
* | da335bec Pretty print the Java language module compiler output
* | 5d831af0 Hook up make pretty printing to the Unit core and tests
* | 280a978d Add initial infrastructure for pretty printing make output
* | c1e3f02f Compile with -fno-strict-overflow
* | 0b5223e1 Disable strict-aliasing in clang by default
* | 1dcb5383 Expand the comment about -Wstrict-overflow on GCC
* | 806e209d Remove -W from compiler flags
* | 9cd11133 Remove support for Sun's Sun Studio/SunPro C compiler
* | e79e4635 Remove support for IBM's XL C compiler
* | 0c2d7786 Remove support for Intel's icc compiler
* | 5511593d Remove support for Microsoft's Visual C++ compiler
* | f6899af6 Var: Fix cacheable issue for njs variable access
* | 63bc3882 .mailmap: Map Dylan's 2nd GitHub address
* | 0cee7d1a Add GitHub workflow for wasm-wasi-component
* | 8032ce31 Test with root access in GitHub workflows
* | c2f7f296 Avoid potential NULL pointer dereference in nxt_router_temp_conf()
* | 353d2d05 Var: Remove a dead assignment in nxt_var_interpreter()
* | 4eb008bb Remove unused nxt_vector_t API
* | 8ff606fb Configuration: Fix check in nxt_conf_json_parse_value()
* | 23e807de Wasm-wc: use more common uname switch to get operating system name
* | e67d7433 Version bump
|/  
* 08811700 Added version 1.32.0 CHANGES

Spot the duplicates. It's not so bad in this case as there is only three of them, it was way worse with the 1.29.1 back-merge.

Someone just looking at this may wonder what the story is, on the face of it they look maybe the same, same subject, message, author, but you'd need to actually compare the diff's to be sure...

ac000 commented 3 months ago

For comparison here's the 1.29.1 back-merge...

* | 814815a3 Merged with the 1.29 branch.
|\| 
| * d5382aeb Unit 1.29.1 release.
| * 0af1253c (tag: 1.29.1) Generated Dockerfiles for Unit 1.29.1.
| * 32f9c3d6 Added version 1.29.1 CHANGES.
| * 8295a0eb Changes moved to the correct section.
| * ca998817 Added missing fixes in changes.xml.
| * f4298180 contrib: updated njs to 0.7.10.
| * 29471c8d Set a safer umask(2) when running as a daemon.
| * 5c9113dd Isolation: rootfs: Set the sticky bit on the tmp directory.
| * 1b7cf1f3 Tests: added Python tests with encoding.
| * 7d7b5a97 Remove the nxt_getpid() alias.
| * 58812e74 Isolation: Remove the syscall(SYS_getpid) wrapper.
| * b9c1a297 Isolation: Remove nxt_clone().
| * c1299faa Isolation: Switch to fork(2) & unshare(2) on Linux.
| * a83354f4 Enable the PR_SET_CHILD_SUBREAPER prctl(2) option on Linux.
| * b7f1d725 Isolation: Rename NXT_HAVE_CLONE -> NXT_HAVE_LINUX_NS.
| * 0277d8f1 Isolation: Fix the enablement of PR_SET_NO_NEW_PRIVS.
| * 1f37d812 NJS: adding the missing vm destruction.
| * b2d19155 Python: ASGI: Don't log asyncio.get_running_loop() errors.
| * 843ae1b8 Python: ASGI: Switch away from asyncio.get_event_loop().
| * 21bc8ee1 Python: ASGI: Factor out event loop creation to its own function.
| * 420ddd4e PHP: Fix a potential problem parsing the path.
| * a8dde6a5 Fix endianness detection in nxt_websocket_header_t.
| * c175e47c Autodetect endianness.
| * 96891a30 Python: Fix enabling of UTF-8 in some situations.
| * fcbb26e9 Python: Do some cleanup in nxt_python3_init_config().
| * 2596a895 Packages: do not clean up rpm build root.
| * 2a05b207 Docs: added changelog for Python 3.11.
| * b5952edc Version bump.
* | 32c9b8c3 Added missing fixes in changes.xml.
* | 132f7d18 contrib: updated njs to 0.7.10.
* | bbeccabe Tools: improved detection of unitd control socket.
* | 5ed6eae7 Set a safer umask(2) when running as a daemon.
* | ffa86b6e Isolation: rootfs: Set the sticky bit on the tmp directory.
* | 7934dcab Tests: switched to using f-strings.
* | fcabbf09 Tests: added Python tests with encoding.
* | 7f046c80 Tests: removed list usage as default argument.
* | e249dd47 Tools: using nicer characters for showing a tree.
* | 96f33c03 Remove the nxt_getpid() alias.
* | 3351bb4f Isolation: Remove the syscall(SYS_getpid) wrapper.
* | 1388d311 Isolation: Remove nxt_clone().
* | b0e2d9d0 Isolation: Switch to fork(2) & unshare(2) on Linux.
* | d98a1b0d Enable the PR_SET_CHILD_SUBREAPER prctl(2) option on Linux.
* | 3ecdd2c6 Isolation: Rename NXT_HAVE_CLONE -> NXT_HAVE_LINUX_NS.
* | 763396b8 Isolation: Fix the enablement of PR_SET_NO_NEW_PRIVS.
* | b0bb8291 Packages: get rid of deprecated configure options.
* | 789762ff NJS: adding the missing vm destruction.
* | cbc01907 Python: ASGI: Don't log asyncio.get_running_loop() errors.
* | 9c0a4a09 Python: ASGI: Switch away from asyncio.get_event_loop().
* | 4b7a9546 Python: ASGI: Factor out event loop creation to its own function.
* | 5a37171f Added default values for pathnames.
* | 0be289b7 Tests: Add some PHP tests for 403 and 404 error handling.
* | bebc03c7 PHP: Implement better error handling.
* | fafdb7a5 PHP: Simplify ctx->script_filename.start in nxt_php_execute().
* | 3923de98 PHP: Make use of zend_stream_init_filename().
* | 0686740f PHP: Factored out code into a helper function.
* | 05c56394 Tests: added NJS iteration tests.
* | 6dad38f6 Tests: NJS tests reworked.
* | 3d930a7c Removed repetitive phrasing from README.
* | 71c2db15 Liam.
* | 141deec3 NJS: added the keys API for the request objects.
* | 97caab0e PHP: Fix a potential problem parsing the path.
* | 2c7e1bb9 Fix endianness detection in nxt_websocket_header_t.
* | ead3580d Autodetect endianness.
* | f3d05bba Python: Fix enabling of UTF-8 in some situations.
* | a560cbf9 Python: Do some cleanup in nxt_python3_init_config().
* | 834c1a2d Fixed the Slack workspace link.
* | 88b04f3e Tools: setup-unit: disabled buggy behavior of zsh(1).
* | 2435bd1c Tools: setup-unit: workarounded macOS tmp file permissions.
* | 6861c25e Tools: setup-unit: removed root checks.
* | ab3d1297 Packages: do not clean up rpm build root.
* | 24d4dda2 Docs: added changelog for Python 3.11.
* | a2b39924 Tools: unitc avoid interactive rm(1) invocations.
* | 9c94cfcc Tools: Fixed bug in help message.
* | 99a7fa78 Tools: Using HereDoc instead of echo(1).
* | aaba6fdc Version bump.
* | e2dd3610 Tools: fixed quoting for apostrophe in setup-unit.
* | 4409a10f Unit 1.29.0 release.
|/  
* 87a1a9c0 (tag: 1.29.0) Generated Dockerfiles for Unit 1.29.0.
andrey-zelenkov commented 3 months ago

They also skew repository statistics.

What statistics and why it's important?

They could also raise some questions about "Which one do I revert?"

git blame provides the perfect answer for this and does the job for you. I just checked the result for the Docs: added changelog for Python 3.11 commit and it shows only one revision:

% git blame docs/changes.xml -L 626,628
24d4dda25 (Konstantin Pavlov 2022-12-15 08:37:52 -0800 626) <para>
24d4dda25 (Konstantin Pavlov 2022-12-15 08:37:52 -0800 627) Initial release of Python 3.11 module for NGINX Unit.
24d4dda25 (Konstantin Pavlov 2022-12-15 08:37:52 -0800 628) </para>
alejandro-colomar commented 3 months ago

Hi @andrey-zelenkov ,

They could also raise some questions about "Which one do I revert?"

git blame provides the perfect answer for this and does the job for you. I just checked the result for the Docs: added changelog for Python 3.11 commit and it shows only one revision:

% git blame docs/changes.xml -L 626,628
24d4dda25 (Konstantin Pavlov 2022-12-15 08:37:52 -0800 626) <para>
24d4dda25 (Konstantin Pavlov 2022-12-15 08:37:52 -0800 627) Initial release of Python 3.11 module for NGINX Unit.
24d4dda25 (Konstantin Pavlov 2022-12-15 08:37:52 -0800 628) </para>

Some features of git get confused by such merges. For example, I had a problem a few days ago checking which was the tag that contains a commit in a project.

Let's say I want to know which is the first tag that contains the commit bbeccabe.

alx@debian:~/src/nginx/unit/master$ git describe --contains bbeccabe
fatal: cannot describe 'bbeccabe0a40c689466d97570e44829e3c1ddb3b'

Hmm, the problem comes from the merge from 1.29.1 to the master branch, so there are two branches in parallel, and git can't describe unambiguously this commit as being contained by one tag. Admittedly, this ambiguity comes from every merge commit, I think. That's not to say that every merge is bad, but it's not innocuous either; they should be used judiciously.

It can be checked that 1.30.0 contains this commit, and that no previous tags contain it:

alx@debian:~/src/nginx/unit/master$ git log --oneline 1.30.0 | grep bbeccabe
bbeccabe Tools: improved detection of unitd control socket.
alx@debian:~/src/nginx/unit/master$ git log --oneline 1.29.1 | grep bbeccabe
alx@debian:~/src/nginx/unit/master$ git log --oneline 1.29.0 | grep bbeccabe
alx@debian:~/src/nginx/unit/master$ 

But git got confused by the merge.

If we look at a commit that doesn't suffer such a merge, git works just fine:

alx@debian:~/src/nginx/unit/master$ git describe --contains faa7e792
1.32.0~5

So, leaving the history simple seems the better option to me.

If you remember, we had this issue a while back with 1.29.0 and 1.29.1

I remember this and I still think that crafting this commit manually is greater evil than a mere merge (since I don't see anything bad in duplicated commits).

Do nothing and treat master and 1.32 as totally separate entities.

This leave us with inconsistent CHANGES at least, that was reported by @tippexs and @Jcahilltorre

Could you describe what's the inconsistency? I maintain the stable branches of shadow, at https://github.com/shadow-maint/shadow/tree/4.14.x and https://github.com/shadow-maint/shadow/tree/4.14.x and use the strategy that @ac000 described, and never had problems with it. Here's (part of) the log:

alx@debian:~/src/shadow/stable/shadow/master$ git log --oneline --graph 4.14.x 4.15.x master
* 7ceeec8d (tag: 4.14.7, stable/4.14.x, shadow/4.14.x, 4.14.x) Release 4.14.7
* aed99b13 lib/copydir.c: copy_entry(): Use temporary stat buffer
* 89d26e03 man/po/fr.po: Fix wrong french translation
* f4293f9f lib/, src/: Add checks for fd omission
* 39192107 src/vipw.c: Use string literals to initialize 'Prog'
* 470d6be2 src/vipw.c: Reverse logic and variable name
* a2837133 src/: Hardcode Prog to known value
* 71080e79 (tag: 4.14.6) Release 4.14.6
* 7e396ba4 lib/utmp.c: Use the appropriate autotools macros for struct utmpx
* 128fe119 lib/utmp.c: Use defined() instead of #if[n]def
* 2da400de lib/utmp.c: Remove #endif comments
* 561cbbe8 lib/utmp.c: Merge preprocessor conditionals
* 261f4042 lib/utmp.c: Indent nested preprocessor conditionals
* 02a9d041 lib/utmp.c: Replace UT_LINESIZE by a NITEMS() calculation
* bec925d2 lib/, src/, configure.ac: Use utmpx instead of utmp
* 2128715e Revert 3 cherry-picks
* fee5e61d lib/getdate.y: get_date(): Fix calculation
* 9d5591fb src/passwd.c: check password length upper limit
* bed23cc3 src/passwd.c: inconsistent password length limit
* c4eae354 lib/strtoday.c: strtoday(): Fix calculation
* d6a9b726 src/login.c: Fix off-by-one bugss
* cc2970c3 src/login.c: Fix off-by-one buggs
* dbdda2a4 lib/: Saturate addition to avoid overflow
* 541d4dde src/chage.c: Unify long overflow checks in print_day_as_date()
* 55f9635e lib/, src/: Remove SCALE definition
* 25fd8eb4 lib/defines.h: Remove ITI_AGING
* 24605a1b (tag: 4.14.5) Release 4.14.5
* 9f3d42b1 etc/pam.d/Makefile.am: Fix typo
* f0f7fc60 (tag: 4.14.4) Release 4.14.4
* bc0151d4 lib/chkname.c: Take NUL byte into account
* 4b775cbf lib/chkname.c: Use tmp variable to avoid a -Wsign-compare warning
* bc2cc110 Makefile: Move chpasswd and newusers to pamd target
* f630203e lib/logind.c: active_sessions_count(): Fix build error 'parameter name omitted'
* 7540b051 Link correctly with libdl
* eae0b027 (tag: 4.14.3) Release 4.14.3
* 1c6a1206 lib/sgetgrent.c: fix null pointer dereference
* 22656c36 (tag: 4.14.2) Release 4.14.2
* 11071522 man: document --prefix option in chage, chpasswd and passwd
* 909036d7 useradd: Set proper SELinux labels for def_usrtemplate
* de50b394 utmp: call prepare_utmp() even if utent is NULL
* b5c99ec3 lib/btrfs: avoid NULL-dereference
* 58b96645 Replace __{BEGIN,END}_DECLS with #ifdef __cplusplus
* 817f3283 (tag: 4.14.1) Release 4.14.1
* 0e0bcacf (tag: 4.14.1-alx) Release 4.14.1-alx
* 0004cc46 (tag: 4.14+alx4.14.1-rc1) lib: Merge libmisc into libshadow
* 1c330177 lib, libmisc: Move source files to lib (where their headers were)
| * dc12e87f (HEAD -> master, tag: 4.15.1, stable/master, stable/4.15.x, shadow/master, shadow/4.15.x, alx/master, 4.15.x) configure.ac: release 4.15.1
| * 4827da0a src/login.c: Use localtime_r(3) instead of localtime(3)
| * 0460dac0 lib/, src/: Use STRFTIME() instead of its pattern
| * b3affb29 lib/string/strftime.[ch]: STRFTIME(): Add macro
| * 0b3d0172 man/Makefile.am: ship config.xml
| * e08db2de man/po/Makefile.in: avoid unnecessary changes to git indexed files
| * 55c10761 update translations
| * 673ff74f Makefile.am: clean some tempfiles
| * ead55e9b getdef: avoid spurious error messages about unknown configuration options
| * 00061934 lib/copydir:copy_entry(): use temporary stat buffer
| * 51a0d94a Fix wrong french translation
| * e44a9e63 gitignore: add a few more generated files to be ignored
| * 2b67dc77 lib/pam_pass_non_interactive.c: use strzero/free
| * fce1d884 lib/list.c: is_on_list(): Call strsep(3) instead of open-coding it
| * 46fd68c3 lib/list.c: is_on_list(): Move break condition to loop controlling expression
| * fb01e07e lib/list.c: is_on_list(): Move code out of loop
| * 08ae38e3 lib/list.c: is_on_list(): Remove unnecessary use of temporary variable
| * 34b113ba lib/sgetspent.c: sgetspent(): Explicitly use an empty string literal
| * 93151689 lib/sgetspent.c: sgetspent(): Use NULL instead of 0 to mean a null pointer constant
| * ae17e029 lib/port.c: getportent(): Call strpbrk(3) instead of open-coding it
| * 03677d9a lib/: Call strsep(3) instead of open-coding it
| * 5f8f19f2 lib/: Call strchrnul(3) instead of open-coding it
| * bed18501 lib/, src/: Call gmtime_r(3) instead of gmtime(3)
| * 8fcf6ccc lib/time/day_to_str.[ch]: day_to_str(): Accept a day instead of a date, and rename function
| * 8fee869e src/passwd.c: print_status(): Fix typo (bogus use of the comma operator)
| * 82e28ad5 src/: Use DAY_TO_STR() instead of its pattern
| * 19edb06f lib/time/day_to_str.h: DAY_TO_STR(): Add macro
| * be05c62b lib/, src/, po/: date_to_str(): Move function to header, and make inline
| * 88760598 src/sulogin.c: Invert logic to reduce indentation
| * efd169e0 lib/, src/: Use int main(void) where appropriate
| * da440b53 lib/: Clean up after previous removal of dead code
| * 33825ab5 lib/, src/: Remove all code wrapped in defined(USE_NIS)
| * ae3d71fb src/passwd.c: Don't print the program name twice in a log entry
| * 4959cd10 Noting copy_symlink behaviour
| * a3cae72f share/containers/, .github/workflows/: Don't make(1) twice
| * 26deef69 lib/idmapping.c: get_map_ranges(): Merge two input checks into a simpler one
| * d2f2c187 Adding checks for fd omission
| * b76fc294 tests/unit/test_zustr2stp.c: Test ZUSTR2STP()
| * ffb39924 lib/string/zustr2stp.[ch]: Remove zustr2stp(); keep ZUSTR2STP()
| * ba43b49a (tag: 4.15.0) configure.ac: Release 4.15.0
| * 89c4da43 src/vipw.c: Use string literals to initialize 'Prog'
| * 0ab893a7 src/vipw.c: Reverse logic and variable name
| * e6c2e439 Hardcoding Prog to known value
| * d1384440 share/containers/: trap(1) to see the cmocka logs
| * e59a3966 share/containers/: Specify one argument per line
| * a14936cf .github/workflows/runner.yml: trap(1) to see the testsuite log
...
| * f76c31f5 Avoid usage of sprintf
| * e0d3ba69 commonio: check for path truncations
| * 54ab5428 lib/btrfs: avoid NULL-dereference
| * a08021eb lib/commonio: drop dead store
| * 931e7c0c login: use strlcpy to always NUL terminate
| * 15f4421f lib: avoid dropping const qualifier during cast
| * 856ffcfa Drop unnecessary cast to same type
| * 35edae58 Declare usage and failure handler noreturn
| * 1aaa4ec5 lib/tcbfuncs: operate on file descriptor rather than path
| * f45498a6 libmisc/write_full.c: Improve write_full()
| * 890f911e Replace __{BEGIN,END}_DECLS with #ifdef __cplusplus
|/  
* 014536f5 (tag: 4.14.0) release 4.14.0
* ca0f828e (tag: 4.14.0-rc5) pre-release 4.14.0-rc5

Have a lovely day, Alex

ac000 commented 3 months ago

Hi Alex,

alx@debian:~/src/nginx/unit/master$ git describe --contains bbeccabe fatal: cannot describe 'bbeccabe0a40c689466d97570e44829e3c1ddb3b'

Hmm, that seems to work for me...

$ git --version
git version 2.44.0
$ git describe 
1.32.0-47-ge75f8d5d
$ git describe --contains bbeccabe
1.30.0~80

But yes, maybe that tickled a bug that got fixed....

Doing things which can confuse humans (and git) is not great...

That's not to say that every merge is bad, but it's not innocuous either; they should be used judiciously.

Indeed.

A project like Linux would not work without them. But even then Linus will shout at you if you don't explain why the merge commit is needed.

@andrey-zelenkov

I'm curious how we would explain the need to merge bugfixes back into master that already exist there?

ac000 commented 3 months ago

This leave us with inconsistent CHANGES https://github.com/nginx/unit/blob/master/CHANGES at least, that was reported by @tippexs and @Jcahilltorre

I'm evermore convinced that the changelog files should simply be ignored until release time.

If we want the bugfix release mentioned in the master changelog then simply make a commit to add them...

alejandro-colomar commented 3 months ago

Hi Andrew,

Hmm, that seems to work for me...

$ git --version
git version 2.44.0
$ git describe 
1.32.0-47-ge75f8d5d
$ git describe --contains bbeccabe
1.30.0~80

But yes, maybe that tickled a bug that got fixed....

Hmm, here on Debian Sid we're still on git 2.43. That might be it.

This leave us with inconsistent CHANGES https://github.com/nginx/unit/blob/master/CHANGES at least, that was reported by @tippexs and @Jcahilltorre

I'm evermore convinced that the changelog files should simply be ignored until release time.

If we want the bugfix release mentioned in the master changelog then simply make a commit to add them...

If that's the problem... I'd say it's not a problem. bugfix releases are not part of the main branch, and so should not be mentioned. They would only confuse users. Let me explain:

If a user sees the changelog, and sees that version 1.29.999 fixed a bug, and the user is on 1.30.0, should they expect the bug to be fixed or not? Is it even relevant at all for 1.30.0 to know what happened on the 1.29.x branch? Maybe 1.29.999 is released after 1.30.0; or maybe the maintainers didn't patch 1.30.x because they didn't notice it was also vulnerable. They are orthogonal, from the point they branched away.

ac000 commented 3 months ago

I'm 50/50 on whether we should list bugfix releases in the master changelog.

Then again I'm generally doubtful about having the changelog, in the repository at least, at all...

I remember this was a hot topic when git was first being adopted by projects, the question of whether traditional changelogs were still required seeing as you can get all the information out of git-log(1).

andrey-zelenkov commented 3 months ago

I'm curious how we would explain the need to merge bugfixes back into master that already exist there?

CHANGES file used as a reference during release as a source of recent changes. Therefore, the fact of the release should be noted in the master branch.

For now I have a simple task: to transfer changes from one branch to another. The very first sentence from the git merge documentation states: "Incorporates changes from the named commits (since the time their histories diverged from the current branch) into the current branch." That's exactly what I am looking for here.

ac000 commented 3 months ago

CHANGES https://github.com/nginx/unit/blob/master/CHANGES file used as a reference during release as a source of recent changes. Therefore, the fact of the release should be noted in the master branch.

This can go either way. As Alex mentioned, the 1.32 branch can be thought of as being distinct from master, with its own distinct history and changelog.

For now I have a simple task: to transfer changes from one branch to

Which changes exactly?

another. The very first sentence from the git merge documentation states: "Incorporates changes from the named commits (since the time their histories diverged from the current branch) into the current branch." That's exactly what I am looking for here.

Yes. That's the general idea and that's not in dispute.

What I'm more talking about is why merge bugfixes back into master that are already there?

See this as an example of a merge commit.

It lists the fixes that are being merged. So we'd list the bugfixes, but hang on, we already have them, so no need to merge them.

This is also somewhat related...

If we did a 1.32.2 bugfix release and then you tried to merge that back into master... I'm not even sure what that'd look like, but I'm sure it won't look pretty...

andrey-zelenkov commented 1 week ago

No longer relevant. Closing.