libwebsockets: update to 4.3.3

[liudongmiao]

Maintainer: OpenWrt.org
Compile tested: ath79-generic-openwrt-23.05 (docker sdk tag)
Run tested: ath79/generic, OpenWrt 23.05.2, Netgear WNDR3800

Description: Update libwebsockets to latest stable 4.3.3, released on 2023/11/20

[BKPepe]

Master first then it could be backported to the stable branches.

[liudongmiao]

@oskarirauta @neheb add two fix for lws-4.3.2 in 6a897e2.

For 010-fix-enum-int-mismatch-openssl.patch, the upstream pr is warmcat/libwebsockets#2824 and is merged in lws-4.3.3.
For 011-fix-enum-int-mismatch-mbedtls.patch, the upstream pr is warmcat/libwebsockets#3027, and upstream said it's fixed already.

@BKPepe Then, how to do with master? Do we need to keep the 011-fix-enum-int-mismatch-mbedtls.patch?

[BKPepe]

My idea:

1. Drop, try to reproduce, if the issue still persists.
2. Keep it, compile it and you will see how it goes
3. You can check the source code and I see that you did not look at it.
   The patch was applied here: warmcat/libwebsockets@a07699d

So, while you will bump this to 4.3.3, you will see that both patches does not apply, because both of them are included in the 4.3.3 release.

You are welcome.

[liudongmiao]

@BKPepe Thanks. (And sorry, I din't check the history of the file in upstream.)

And make a new pr for master: #23566

[liudongmiao]

make it base on master

[neheb]

No need for these two commits. Merge into 1.

[liudongmiao]

@1715173329 As I change the base from openwrt-23.05 to master, could you remove the label "OpenWrt 23.05"?

[liudongmiao]

oskarirauta authored and neheb committed Jul 1, 2023

@oskarirauta @neheb Update libwebsockets 4.3.3 revert your commit 6a897e2 based on 4.3.2 (as there were fixed in 4.3.3), please help to review. And, do we need to add wolfssl variant?

[BKPepe]

I don't know what you are trying right now.

This commit: 2b9b12f was perfectly fine and nothing else was needed except that you need to run: make package/foo/refresh V=s. Refresh does not mean to do revert commit, etc. It was not a complicated step, and now we are trying to reinvent the wheel.

Such a simple change, and now it is not even efficient for simple bumps. :-/

Changing PR from one to another, you could keep this for OpenWrt 23.05 and then rebase it or re-do it with cherry-pick.

Please keep this simple as it is.

[liudongmiao]

@BKPepe Thanks for the make package/foo/refresh V=s, and update.

And, add wolfssl support.

[BKPepe]

The revert commit needs to be gone. Also, as you added WolfSSL variant (why not do that in the different PR, oh boy...)
You need to follow https://openwrt.org/submitting-patches#submission_guidelines , basically you need to add commit description, etc.

[liudongmiao]

@BKPepe Thanks for Review. Just remove the wolfssl, and rebase into one commit.

[BKPepe]

However, i still do have problem that you are using word revert. You dont revert anything. You removed patch, because it was backport as a patch from upstream. Dne út 5. 3. 2024 16:22 uživatel Liu DongMiao ***@***.***> napsal:

…

@liudongmiao <https://github.com/liudongmiao> requested your review on: #23562 <#23562> libwebsockets: update to 4.3.3. — Reply to this email directly, view it on GitHub <#23562 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA7IDVAWJMRLATIPLEOC35LYWXPKHAVCNFSM6AAAAABD7I57F6VHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSGAYTKNRUG43TANA> . You are receiving this because your review was requested.Message ID: ***@***.***>

[liudongmiao]

@BKPepe Thanks, I use other words to avoid reverts:

The old patches are removed as there were fixed in 4.3.3.

[BKPepe]

Much better. 👏

[liudongmiao]

@BKPepe current failed either in build image, or in depends. I think it's nothing due to the libwebsockets itself.

And I add two more commits, as it's not merged yet.

• make openssl variant provides libwebsockets
• make all variants requires libuv, its default for distros like debian, ubuntu, fedora, centos, arch linux and macos

[BKPepe]

remove conflicts as similar packages don't specify at all.

This is really not true at all. Did you figure out why there are conflicts and what is their purpose? It does not mean that those are not wrong. Please check this out: #19701

[liudongmiao]

IMO, package should avoid to require specify variant.

mosquitto-ssl requires libwebsockets-openssl, and ttyd requires libwebsockets-full.
If there's other package requires libwebsockets, don't care about the variants at all, then which they should install?

Of course, if it's the real case, then make libwebsockets-full provides libwebsockets and extra libwebsockets-openssl.

And if libwebsockets-full provides libwebsockets-openssl, then it's ok:

root@OpenWrt:~# opkg list-installed | grep -E '(libwebsockets|ttyd|mosquitto-ssl)'
libwebsockets-full - 4.3.3-1
mosquitto-ssl - 2.0.17-1
ttyd - 1.7.3-1

[BKPepe]

Anyway, this is really getting huge and complicated. What are you doing is now that all your changes even though splitted in multiple commits are in pull request, which should only update it to 4.3.3. So it gets really difficult to review it and as well to get in. There was a reaosn, why it was not merged, because it was waiting for CI/CD to finish its job.

Also, if you are adding more commits, which are not relevant to PR, then it is get messy, because of the noise. Everyone could review different commits, give you feedback and it keeps reminder others, which might not be interested in other commits.

Please try to keep this simple as possible.

1. Update: OK
2. Removing conflicts: not OK
3. Not understand the 3rd commit.

[liudongmiao]

@BKPepe For the third commit, remove the -DLWS_WITH_PLUGINS=ON.

It's because it's not binary compatible if we enable LWS_WITH_PLUGINS, refer:

https://github.com/warmcat/libwebsockets/blob/v4.3.3/include/libwebsockets/lws-context-vhost.h#L646-L650

It takes me 3 days to find the why my codes doesn't work with specify variant.

There are two solutions:

• For all variants, add -DLWS_WITH_PLUGINS=ON
• For all variants, remove -DLWS_WITH_PLUGINS=ON
  As other distros don't add -DLWS_WITH_PLUGINS=ON, so I choose to remove.

[liudongmiao]

@BKPepe I just remove other (conflicts / provides, libuv, -DLWS_WITH_PLUGINS=ON) in the PR.
Yes, it should be discussing in other PR, not this one.

If it's merged, then I will add other PR to discussing them.