-
Notifications
You must be signed in to change notification settings - Fork 337
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
Merged with the 1.32 branch #1205
Conversation
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
Reproduces issue nginx#1169.
Liam reported a problem when trying to restart wasm-wasi-component based applications using the /control/applications/APPLICATION_NAME/restart endpoint. The application would become unresponsive. What was happening was the old application process(es) weren't exit(3)ing and so while we were starting new application processes, the old ones were still hanging around in a non-functioning state. When we are terminating an application it must call exit(3). So that's what we do. We use the return value of nxt_unit_run() as the exit status. Due to exit(3)ing we also need to now explicitly handle the return on error case. Reported-by: Liam Crilly <[email protected]> Fixes: 20ada4b ("Wasm-wc: Core of initial Wasm component model language module support") Closes: nginx#1179 Tested-by: Liam Crilly <[email protected]> Tested-by: Danielle De Leo <[email protected]> Co-developed-by: Dan Callahan <[email protected]> Signed-off-by: Dan Callahan <[email protected]> Signed-off-by: Andrew Clayton <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will give us duplicate commits again...
I'm afraid this is also wrong. We will end up with the three bugfix commits that are already on 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 I see two options.
|
version
Outdated
NXT_VERSION=1.32.0 | ||
NXT_VERNUM=13200 | ||
NXT_VERSION=1.32.1 | ||
NXT_VERNUM=13201 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current version is 1.33.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know where it came from, but it's not part of this PR
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).
This leave us with inconsistent CHANGES at least, that was reported by @tippexs and @Jcahilltorre |
Duplicate commits clutter and pollute the history reducing its They could also raise some questions about "Which one do I revert?" We would now have a history like
Spot the duplicates. It's not so bad in this case as there is only three Someone just looking at this may wonder what the story is, on the face |
For comparison here's the 1.29.1 back-merge...
|
What statistics and why it's important?
|
Hi @andrey-zelenkov ,
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 bbeccab. 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. 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.
Could you describe what's the inconsistency? I maintain the stable branches of shadow, at
Have a lovely day, |
Hi Alex,
Hmm, that seems to work for me...
But yes, maybe that tickled a bug that got fixed.... Doing things which can confuse humans (and git) is not great...
Indeed. A project like Linux would not work without them. But even then Linus I'm curious how we would explain the need to merge bugfixes back into |
I'm evermore convinced that the changelog files should simply be ignored If we want the bugfix release mentioned in the master changelog then |
Hi Andrew,
Hmm, here on Debian Sid we're still on git 2.43. That might be it.
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. |
I'm 50/50 on whether we should list bugfix releases in the master Then again I'm generally doubtful about having the changelog, in the I remember this was a hot topic when git was first being adopted by |
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 |
This can go either way. As Alex mentioned, the 1.32 branch can be
Which changes exactly?
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 See this It lists the fixes that are being merged. So we'd list the bugfixes, but This is also somewhat related... If we did a 1.32.2 bugfix release and then you tried to merge that back |
No longer relevant. Closing. |
No description provided.