Skip to content
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

Revert "fcgi: remove" #19810

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Revert "fcgi: remove" #19810

wants to merge 3 commits into from

Conversation

ynezz
Copy link
Member

@ynezz ynezz commented Nov 4, 2022

Compile tested: OpenWrt 22.03 x86/64, ipq40xx, mvebu/cortexa9
Run tested: OpenWrt 22.03 x86/64, ipq40xx, mvebu/cortexa9

This reverts commit 65ef6c6 where @neheb removed fcgi library for "Nothing uses this." reason.

I've noticed this missing library while rebasing downstream project from OpenWrt 19.07 release base to OpenWrt 22.03, as there is amx-fcgi system component which depends on this library.

So I would like to fix that regression by adding back the removed library package.

@neheb
Copy link
Contributor

neheb commented Nov 5, 2022

I probably also removed it as it was failing in some other way (maybe nls or autoconf issues).

Where is this amx-fcgi used?

@ynezz
Copy link
Member Author

ynezz commented Nov 5, 2022

I probably also removed it as it was failing in some other way (maybe nls or autoconf issues).

I can't find anything regarding this in the issue tracker, it compiles and works fine, CI here doesn't complain either.

Where is this amx-fcgi used?

tr181-rest-api for example.

@ynezz
Copy link
Member Author

ynezz commented Nov 5, 2022

Here is the regression exposed by the latest OpenWrt 22.03.2 x86/64 SDK:

x86_64-openwrt-linux-musl-gcc -Os -pipe -fno-caller-saves -fno-plt -fhonour-copts -Wno-error=unused-but-set-variable -Wno-error=unused-result -ffile-prefix-map=/home/build/openwrt/build_dir/target-x86_64_musl/amx-fcgi-v0.2.3=amx-fcgi-v0.2.3 -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro  -I/home/build/openwrt/staging_dir/toolchain-x86_64_gcc-11.2.0_musl/usr/include -I/home/build/openwrt/staging_dir/toolchain-x86_64_gcc-11.2.0_musl/include/fortify -I/home/build/openwrt/staging_dir/toolchain-x86_64_gcc-11.2.0_musl/include  -Werror -Wall -Wextra -Wformat=2 -Wshadow -Wwrite-strings -Wredundant-decls -Wmissing-declarations -Wno-attributes -Wno-format-nonliteral -fPIC -g3 -I ../include_priv -I /home/build/openwrt/staging_dir/target-x86_64_musl/include -I /home/build/openwrt/staging_dir/target-x86_64_musl/usr/include -Wstrict-prototypes -Wold-style-definition -Wnested-externs -std=gnu11 -c -o ../output/x86_64-openwrt-linux-musl/object/amx_fcgi.o amx_fcgi.c
In file included from ../include_priv/amx_fcgi.h:69,
                 from amx_fcgi.c:64:
../include_priv/amx_fcgi_types.h:69:10: fatal error: fcgiapp.h: No such file or directory
   69 | #include <fcgiapp.h>
      |          ^~~~~~~~~~~
compilation terminated.

It is working fine with OpenWrt 19.07 SDK.

@champtar
Copy link
Member

champtar commented Nov 5, 2022

@ynezz could you maybe add a comment in the makefile pointing to the external users of the lib ? It's a bit weird to keep libraries with no users.
Also should you set yourself as maintainer ?

@ynezz
Copy link
Member Author

ynezz commented Nov 6, 2022

It's a bit weird to keep libraries with no users.

Fair enough, I wasn't aware about this rule.

@ynezz ynezz closed this Nov 6, 2022
@champtar
Copy link
Member

champtar commented Nov 6, 2022

IMO It's fine to keep it in as there is at least one known user, but it's better to make it explicit so we don't delete it again in the future.

@neheb
Copy link
Contributor

neheb commented Nov 6, 2022

I think if introduced it should also be removed from packages-abandoned.

@ynezz
Copy link
Member Author

ynezz commented Nov 7, 2022

IMO It's fine to keep it in as there is at least one known user, but it's better to make it explicit so we don't delete it again in the future.

Do we really want to start adding this kind of build system unrelated "hey, I'm using it, don't delete it" noise into the Makefiles? How often you would need to update it, so you know, that the users are still active? Once a year? IMO it's going to bit rot, folks can simply miss the information and delete the package anyway. It should probably be documented as well.

What about simply assuming, that there always might be some users out there, so don't delete the package in the first place? If you really need to remove it for some reason, then simply make that reason clear in the commit message and allow adding the package back.

@ynezz ynezz reopened this Nov 7, 2022
@ynezz
Copy link
Member Author

ynezz commented Nov 7, 2022

Also should you set yourself as maintainer ?

Done.

ynezz added a commit to ynezz/packages-abandoned that referenced this pull request Nov 7, 2022
This reverts commit a682bf9 as the
package still have some active users and thus is not abandoned.

References: openwrt/packages#19810
Signed-off-by: Petr Štetiar <[email protected]>
@ynezz
Copy link
Member Author

ynezz commented Nov 7, 2022

I think if introduced it should also be removed from packages-abandoned.

Prepared.

@dhewg
Copy link
Contributor

dhewg commented Nov 7, 2022

Not directly related, but:
I've tried to get some prpl hosted firmware files released under a compatible license for OpenWrt. I've used their issue tracker for that some month ago. Now they've shut down access to it.
Apparently they don't care about open source communities, so why should OpenWrt care about them and maintain this without in-tree users?

@ynezz
Copy link
Member Author

ynezz commented Nov 7, 2022

I've tried to get some prpl hosted firmware files released under a compatible license for OpenWrt. I've used their issue tracker for that some month ago.

As you've probably already noticed, those files reside under Intel vendor/member subgroup and are Copyright 2017-2018, Intel Corporation.. Basically prpl itself can't do much about it since unfortunately none of those files are being used/distributed in any form by prpl. Otherwise it would've need to follow IP Policy and thus have FOSS friendly license already.

What prpl can do in this case is to politely ask MaxLinear representatives to consider re-licensing those files, which was done and AFAIK it's currently waiting for final approval from their legal department, but you're hopefully already aware.

Now they've shut down access to it.

I believe, that its not intentional, IMO its just being caused by the ongoing Jira system migration from on-premise to the new cloud hosted instance. Thank you for letting me know, I'll try to ping responsible folks and ask them for a fix.

so why should OpenWrt care about them and maintain this without in-tree users?

IMO OpenWrt is a FOSS project with a clear contribution rules and guidelines, which anyone could reference in exactly such ambiguities.

IIUC then in FOSS projects "Upstream First" philosophy means, that whenever you solve a problem in your copy of the upstream code which others could benefit from, you contribute these changes back upstream, for example you send a patch or open a PR in the upstream project.

IMO thats exactly what I'm doing, I'm trying to upstream a fix for a downstream project breakage, so anyone else using fcgi library in OpenWrt 19.07 based project and migrating to any later versions could get the fix.

@dhewg
Copy link
Contributor

dhewg commented Nov 8, 2022

What prpl can do in this case is to politely ask MaxLinear representatives to consider re-licensing those files, which was done and AFAIK it's currently waiting for final approval from their legal department, but you're hopefully already aware.

I was aware of an initial ping from a few months ago. Final approval sounds better than that though.

I believe, that its not intentional

I checked their website first, but couldn't find any info on any migration, so I assumed it is intentional.

Thanks, let's hope that gets resolved.

@ynezz
Copy link
Member Author

ynezz commented Nov 9, 2022

Thanks, let's hope that gets resolved.

https://prplfoundationcloud.atlassian.net/browse/PCI-16

@dhewg
Copy link
Contributor

dhewg commented Nov 10, 2022

Nice, thanks!
If that got resolved that quick due to you poking around, I urge you to poke some more to get that the desired response from MaxLinear.

libs/fcgi/Makefile Outdated Show resolved Hide resolved
ynezz added 2 commits July 17, 2024 10:20
This reverts commit 65ef6c6 which has
removed fcgi library for "Nothing uses this." reason.

I've noticed this missing library while rebasing downstream project from
OpenWrt 19.07 release base to OpenWrt 22.03, as there is amx-fcgi[1]
system component which depends on this library.

So I would like to fix that regression by adding back the removed
library package.

1. https://gitlab.com/prpl-foundation/components/ambiorix/applications/amx-fcgi
Signed-off-by: Petr Štetiar <[email protected]>
As requested during the review process.

Signed-off-by: Petr Štetiar <[email protected]>
@ynezz ynezz force-pushed the ynezz/return-fcgi branch from e2e4a48 to 904756d Compare July 17, 2024 10:37
Move away from codeload as requested during the review due to ongoing
issues with this service.

Signed-off-by: Petr Štetiar <[email protected]>
@ynezz ynezz force-pushed the ynezz/return-fcgi branch from 904756d to b28f1d5 Compare July 17, 2024 10:42
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.

5 participants