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

lighttpd: update to 1.4.45 (add new modules) #4126

Merged
merged 1 commit into from Mar 22, 2017
Merged

Conversation

@gstrauss
Copy link
Contributor

gstrauss commented Mar 8, 2017

add new modules to build, add restart() to lighttpd.init, file-glob includes in lighttpd.conf

Signed-off-by: Glenn Strauss gstrauss@gluelogic.com


Maintainer: me
Compile tested: MIPS 74K, NetGear WNDR3700v3, OpenWRT master
Run tested: MIPS 74K, NetGear WNDR3700v3, OpenWRT master

Description: add new modules to build, add restart() to lighttpd.init, file-glob includes in lighttpd.conf

@gstrauss

This comment has been minimized.

Copy link
Contributor Author

gstrauss commented Mar 8, 2017

@MikePetullo any guidance you can provide is appreciated. Thanks.

Copy link
Contributor

hnyman left a comment

The commit title is wrong. This commit does not update the version to 1.4.45. Please edit the title to reflect the contents of the commit.

You also need to edit the commit message. You explain the changes in the discussion that does not get stored into the git log, but say nothing in the commit message. Please move the detailed explanations into the commit message.

@@ -77,15 +76,39 @@ else
--without-openssl
endif

ifneq ($(SDK)$(CONFIG_PACKAGE_lighttpd-mod-mysql-vhost),)
ifneq (,$(CONFIG_PACKAGE_lighttpd-mod-authn-gssapi))

This comment has been minimized.

Copy link
@hnyman

hnyman Mar 10, 2017

Contributor

Why do you change the style in the ifnew clauses? starting with comma? Looks strange and is not usual here.

And is there a reason why you drop the SDK condition from the clause?

@hnyman hnyman added the work needed label Mar 10, 2017
@@ -156,6 +179,15 @@ $(eval $(call BuildPlugin,redirect,URL redirection,+PACKAGE_lighttpd-mod-redirec
# Next, permit authentication.
$(eval $(call BuildPlugin,auth,Authentication,,20))
$(eval $(call BuildPlugin,authn_file,File-based authentication,,20))
ifneq (,$(CONFIG_PACKAGE_lighttpd-mod-authn-gssapi))

This comment has been minimized.

Copy link
@flyn-org

flyn-org Mar 10, 2017

Contributor

I do not think this is right. I did not see the option to select this build feature in "make menuconfig" until I removed the ifneq/endif surrounding each of these $(eval $(call BuildPlugin...)) statements (authn_gssapi, authn_ldap, etc.).

I later found that my .config contained "CONFIG_PACKAGE_lighttpd-mod-authn_ldap," even though the Makefile contained "ifneq (,$(CONFIG_PACKAGE_lighttpd-mod-authn-ldap))." Note that one has an underscore (authn_ldap), but the other has a hyphen.

@flyn-org

This comment has been minimized.

Copy link
Contributor

flyn-org commented Mar 10, 2017

Very good contributions, but I concur with the comments from @hnyman. I also attached a comment to the patch about the configuration options.

@gstrauss gstrauss force-pushed the gstrauss:lighttpd branch 2 times, most recently from 43672c2 to c8a5fce Mar 19, 2017
@gstrauss

This comment has been minimized.

Copy link
Contributor Author

gstrauss commented Mar 20, 2017

Force-pushed updated patch.

  • updated commit message, as requested by @hnyman
  • changed back to ifneq ($(SDK)...,) rather than putting comma first. In large systems with which I have worked, we found ifneq (,$(SDK)...) to be easier to read since the ifneq (,...) comes first, but I defer to the convention used here.
  • removed the conditions around the BuildPlugin make macro so that they show up with make menuconfig, as noted by @MikePetullo

Thanks for the feedback!

@flyn-org

This comment has been minimized.

Copy link
Contributor

flyn-org commented Mar 21, 2017

A few things from testing the most recent commit:

@hnyman, judging the new commit message is a bit subjective. If you do not like it, would you mind providing a replacement?

I had to further modify the Makefile to get it to build the modules I select and not build the modules I do not select using "make menuconfig". I pushed my fixes to https://github.com/MikePetullo/packages/blob/lighttpd/net/lighttpd/Makefile. Can you bring these changes back to your proposed merge after reviewing them?

I have not yet tracked down the cause of the two following items:

Building the Kerberos module seems broken:

checking for kerberos5... yes
checking for gss_acquire_cred in -lgssapi_krb5... no
configure: error: gssapi_krb5 headers and/or libs where not found, install them or build with --without-krb5

Does the Kerberos built on OpenWrt lack a feature needed by lighttpd?

Building the LDAP module seems broken:

OpenWrt-libtool: compile: mips-openwrt-linux-musl-gcc -DHAVE_CONFIG_H -DHAVE_VERSION_H -DLIBRARY_DIR="/usr/lib/lighttpd" -DSBIN_DIR="/usr/sbin" -I. -I.. -I/home/mike/Source/openwrt/openwrt/staging_dir/target-mips_34kc_musl-1.1.14/usr/include -I/home/mike/Source/openwrt/openwrt/staging_dir/target-mips_34kc_musl-1.1.14/include -I/home/mike/Source/openwrt/openwrt/staging_dir/toolchain-mips_34kc_gcc-5.3.0_musl-1.1.14/usr/include -I/home/mike/Source/openwrt/openwrt/staging_dir/toolchain-mips_34kc_gcc-5.3.0_musl-1.1.14/include/fortify -I/home/mike/Source/openwrt/openwrt/staging_dir/toolchain-mips_34kc_gcc-5.3.0_musl-1.1.14/include -D_REENTRANT -D__EXTENSIONS__ -I/home/mike/Source/openwrt/openwrt/staging_dir/target-mips_34kc_musl-1.1.14/usr/include -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGE_FILES -Os -pipe -mno-branch-likely -mips32r2 -mtune=34kc -fno-caller-saves -fno-plt -fhonour-copts -Wno-error=unused-but-set-variable -Wno-error=unused-result -msoft-float -mips16 -minterlink-mips16 -iremap /home/mike/Source/openwrt/openwrt/build_dir/target-mips_34kc_musl-1.1.14/lighttpd-1.4.45:lighttpd-1.4.45 -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro -Wall -W -Wshadow -pedantic -std=gnu99 -MT mod_authn_ldap.lo -MD -MP -MF .deps/mod_authn_ldap.Tpo -c mod_authn_ldap.c -fPIC -DPIC -o .libs/mod_authn_ldap.o
mod_authn_ldap.c:4:18: fatal error: ldap.h: No such file or directory
compilation terminated.

@hnyman

This comment has been minimized.

Copy link
Contributor

hnyman commented Mar 21, 2017

@hnyman, judging the new commit message is a bit subjective. If you do not like it, would you mind providing a replacement?

We can live with the new commit message, as it does not claim a version update any more.

@hnyman
hnyman approved these changes Mar 21, 2017
@gstrauss

This comment has been minimized.

Copy link
Contributor Author

gstrauss commented Mar 21, 2017

Thank you, both.

@MikePetullo: should I integrate the changes in your commit and update this pull request? I now see how this was supposed to be done. Thank you for showing me how.

@flyn-org

This comment has been minimized.

Copy link
Contributor

flyn-org commented Mar 22, 2017

I would modify your commit to include my changes and then force-push to your repository again. The folks with merge rights will likely prefer a single merge.

(with feedback from @hnyman and patch additions from @MikePetullo)

Signed-off-by: Glenn Strauss <gstrauss@gluelogic.com>
@gstrauss gstrauss force-pushed the gstrauss:lighttpd branch from c8a5fce to 6e788ac Mar 22, 2017
@gstrauss

This comment has been minimized.

Copy link
Contributor Author

gstrauss commented Mar 22, 2017

I force-pushed an update with @MikePetullo suggested changes. Additionally, i changed the spelling of dependencies in the Makefile for mod_authn_gssapi and mod_authn_ldap so that those modules build. I commented out geoip and memcached from the Makefile since it does not appear that devel/lib packages are available on openwrt. Everything else builds -- everything that shows in make menuconfig (as available modules for lighttpd) now can build successfully when selected.

I think this is finally ready for merge. Thanks, both, for all of your help.

@hnyman hnyman merged commit eb56619 into openwrt:master Mar 22, 2017
@gstrauss gstrauss mentioned this pull request Mar 23, 2017
@hnyman

This comment has been minimized.

Copy link
Contributor

hnyman commented Apr 8, 2017

There was forum discussion that some of plugins had not been properly built for 1.4.45-2 for the 17.01 release: https://forum.lede-project.org/t/is-it-possible-to-downgrade-some-package/2911

I backported this 1.4.45-3 update to the 17.01 branch to see if that helps. Although all plugins build successfully in my own buildhost, the LEDE buildbot fails to build the authn_gssapi module with the same krb5 errors that MikePetullo got above. (But LEDE master buildbot for LEDE master builds thethe same plugin ok)

http://downloads.lede-project.org/releases/faillogs/mipsel_mips32/packages/lighttpd/compile.txt

checking for kerberos5... yes
checking for gss_acquire_cred in -lgssapi_krb5... no
configure: error: gssapi_krb5 headers and/or libs where not found, install them or build with --without-krb5

I think that I will disable the new authn_gssapi module for 17.01 to see if that is the only problem for buildbot.

But there is likely some underlying library path detection problem in the configure script or something like that. That may also surface at the 15.05 build.

@gstrauss

This comment has been minimized.

Copy link
Contributor Author

gstrauss commented Apr 16, 2017

@hnyman the link to compile.txt logs above now appear to indicate failure with lighttpd-mod-trigger_b4_dl. Would you please comment that out from net/lighttpd/Makefile, too, and see if the rest builds for 17.01? Thanks.

@hnyman

This comment has been minimized.

Copy link
Contributor

hnyman commented Apr 18, 2017

compile.txt logs above now appear to indicate failure with lighttpd-mod-trigger_b4_dl.

lighttpd-mod-trigger_b4_dl_1.4.45-2_mipsel_mips32.ipk' failed

Yep. But note that that the failure message comes from the previous 1.4.45-2 package in the 17.01 branch.

I did not disable the new modules in 1.4.45-3 due to the gssapi/krb5 errors, but instead I rolled back the update to 1.4.45-3 in 17.01 so that there is again the "original" 1.4.45-2 Makefile. Apparently the trigger_b4_dl module has been failing since February in 17.01 and that seems to cause also the modules that are alphabeticallly after that to be left unbuilt (userdir, usertrack and webdav have been compiled successfully last time in early February in the 17.01 buildbot).

Would you please comment that out from net/lighttpd/Makefile, too, and see if the rest builds for 17.01?

Yep, the simple option might be to just disable trigger_b4_dl in 1.4.45-2 in 17.01 branch and see if the missing three modules will then build ok. We can try that. (EDIT: I pushed this change to Github)

Other option might be to upgrade again to 1.4.45-3, but to disable the gssapi authn and possibly other some other plugins, too. trigger_b4_dl has changed dependencies in 1.4.45-3, so a fix for trigger_b4_dl might be there.

@gstrauss

This comment has been minimized.

Copy link
Contributor Author

gstrauss commented Apr 18, 2017

Yep, the simple option might be to just disable trigger_b4_dl in 1.4.45-2 in 17.01 branch and see if the missing three modules will then build ok. We can try that. (EDIT: I pushed this change to Github)

Okay. That's a good starting point.

Other option might be to upgrade again to 1.4.45-3, but to disable the gssapi authn and possibly other some other plugins, too. trigger_b4_dl has changed dependencies in 1.4.45-3, so a fix for trigger_b4_dl might be there.

Yes, my changes to net/lighttpd/Makefile in 1.4.45-3 attempted to fix various dependency issues, including trigger_b4_dl. I am not sure why mod_authn_gssapi fails to build in LEDE 17.01 since it built for you and me (on master). I'll try to reproduce it when I have a chance.

In the meantime, if 1.4.45-2 with trigger_b4_dl succeeds in LEDE 17.01, then the next step would be 1.4.45-3 with trigger_b4_dl enabled, but authn_gssapi disabled, so that any other issues can be identified.

Thanks!

@hnyman

This comment has been minimized.

Copy link
Contributor

hnyman commented Apr 18, 2017

When trigger_b4_dl module is disabled, all other modules seem to build now in 17.01 with 1.4.45-2.
Buildbot is still crunching the targets, but based on the first targets it looks good.
E.g. http://downloads.lede-project.org/releases/17.01.0/packages/arm_mpcore_vfp/packages/ and
http://downloads.lede-project.org/releases/17.01.0/packages/powerpc_8540/packages/

I am not sure why mod_authn_gssapi fails to build in LEDE 17.01 since it built for you and me (on master). I'll try to reproduce it when I have a chance.

This kind of problems may be rather complex to find. One typical reason is due to buildbot building all packages and all libraries. A package's configure script may find some functionality (that is not in your or my test builds) and activates/toggles some feature option. The error may also be in krb5, as the error seems to indicate that krb5 is found but does not contain all needed functionality.

Note that the error happened also for MikePetullo , but he may be using some ancient toolchain (based on musl-1.1.14 that has been deprecated in Openwrt in August 2016 and in LEDE in July 2016).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.