search

erlyaws / yaws

Yaws webserver
https://erlyaws.github.io
BSD 3-Clause "New" or "Revised" License
1.28k stars 267 forks source link

Is there any reason to keep yaws_sendfile_drv.c? #353

Closed leoliu closed 5 years ago

leoliu commented 5 years ago

Erlang has file:sendfile/5 since R15 so I am wondering if it is time for yaws_sendfile_drv.c to go?

Keeping yaws_sendfile_drv.c has some downsides. It is painful when using embedded Yaws because yaws_sendfile starts it even when it is not needed. It needs extra care when using yaws as rebar3 deps.

At the moment I have a complex setup to deploy to centos from macOS and the setup can be reduced to one line if yaws_sendfile_drv.c is no more.

vinoski commented 5 years ago

True, it's probably time to retire this. I'll look into it.

leoliu commented 5 years ago

Hi @vinoski, can this be done? I am dying to be able to pull Yaws as a rebar3 dep without much hassle. I think this is critical for people new to Yaws get started as well.

vinoski commented 5 years ago

I started working on it but I haven't completed it.

leoliu commented 5 years ago

@vinoski, thanks for the update.

vinoski commented 5 years ago

The only current issue with this work is that some versions of R16 had buggy sendfile support. However, I believe the Yaws policy is to support the five most recent Erlang/OTP releases, so technically we can retire support for R16. I'll have to do that first.

leoliu commented 5 years ago

Sounds good. Having OTP 17 as minimal requirement opens up other opportunities such as maps.

vinoski commented 5 years ago

@leoliu please have a look at the drop-yaws-sendfile branch, which currently passes Travis and builds correctly with rebar on Ubuntu. Feedback welcomed.

leoliu commented 5 years ago

This is excellent news to the project! Thanks for the work.

I just added this line to rebar.config in a project and it is working like a charm. No complaints so far.

{yaws, {git, "https://github.com/klacke/yaws.git", {branch, "drop-yaws-sendfile"}}}
vinoski commented 5 years ago

This is now rebased to master, commit 0c700a7c.

leoliu commented 5 years ago

@vinoski I just noticed one small issue. I now use Yaws as a rebar3 dep and after building a release the yaws-2.0.6 directory looks like this:

├── ebin
├── include
└── priv

all these subdirectories have a file Makefile.am which is unusual.

vinoski commented 5 years ago

There's an outstanding issue #262 that was opened a long time ago to add support for rebar3. It's been dormant for awhile but I just rebased master to the rebar3-support branch and pushed it here. I'd like to make changes specific to rebar3 on that branch, so can you try it and if the Makefile.am issue above is still a problem there, raise a new issue for it? Thanks.

leoliu commented 5 years ago

Same issue with rebar3-support. See #368.