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

HTTP TPC v2 #737

Merged
merged 12 commits into from
Jun 15, 2018
Merged

HTTP TPC v2 #737

merged 12 commits into from
Jun 15, 2018

Conversation

bbockelm
Copy link
Contributor

@bbockelm bbockelm commented Jun 8, 2018

This is a renewed version of the TPC code. The core algorithms and approaches are identical but improvements include:

  • File naming follows Xrootd naming scheme.
  • Backport from C++11 to C++03
  • Implement an equivalent to curl_multi_wait for platforms with very old curl.

With this, the TPC code should function on SL6.

…c-rename""

This reverts commit d46c516.

Reverting-the-revert to start working on cleanup of this branch for C++03
support and eventual re-merge.
When an error occurs during a multi-part response, send a chunk-encoded
response and not a complete HTTP response.
With this, functionality on RHEL7 is back to normal.
This is interpretted as just needing one.
@ffurano
Copy link
Contributor

ffurano commented Jun 14, 2018

Hi,

manually on my Ubuntu machine it compiles fine, it only misses to add its item to the little summary printed in the cmake phase. As it's an XrdHTTP plugin I'd list under it.

On a build system for sl6/cc7/x86_64/i386 the TPC is not compiled, as the needed libcurl is not listed in the deps of the specfile, so my nightly test machines are still without it.

Brian, is there any special requirement for libcurl in the specfiles ? Would you mind to fix these two little things?

@abh3
Copy link
Member

abh3 commented Jun 14, 2018

Umm, this pull request violates EPEL packaging rules. A shared library cannot also serve as a plugin library. It's either one or the other but not both. So, the packaging here needs to be rethought. I don't have an immediate solution but I'm sure there is one.

@abh3
Copy link
Member

abh3 commented Jun 14, 2018

Here is a suggestion that would likely work:
a) Keep LIB_XRD_HTTP as a MODULE (maybe the variable should be renamed PLUGIN_XRD_HTTP.
b) In this module only include XrdHttpProtocol (you may want to research to see what other object files are only referenced by XrdHttpProtocol and include those as well).
c) Anything left over goins into libXrdHttpUtils.so (notice this does not have a plugin suffix).
d) The plugin modulewould also link against libXrdHttp.so.
Doing it this way keeps backward compatibility and adheres to EPEL packaging rules.

@bbockelm
Copy link
Contributor Author

@abh3 - are you sure it's an EPEL packaging rule? The closest I can see are the Devel Packages section here: https://fedoraproject.org/wiki/Packaging:Guidelines.

This clearly discourages using an unversioned shared library, but it says nothing against having one module link against another module.

Another route, perhaps less-disruptive, would be to load libXrdHttp.so from the plugin itself. That'd work here, but would clearly discourage other plug-in writers...

The structure of the extension handlers is such that they must link
against some of the XrdHttp code.  Previously, the only way to do this
was to link against the shared module itself.

With this, we create and publish a proper versioned utility library.
@xrootd-dev
Copy link

xrootd-dev commented Jun 14, 2018 via email

@bbockelm
Copy link
Contributor Author

In retrospect, having a versioned interface will help plugin library writers manage version changes in the future.

I just pushed a commit to refactor things into a separate utilities library and touch-up the appropriate packaging. Any external packages that currently link against libXrdHttp.so should continue to function.

@xrootd-dev
Copy link

xrootd-dev commented Jun 14, 2018 via email

@bbockelm
Copy link
Contributor Author

@ffurano - I added the TPC build status to the cmake summary.

For libcurl, you just need the standard library headers (provided by the libcurl-devel RPM). I backported some functionality such that the TPC should function even with the very old version in RHEL6. Additionally, I just added the corresponding build requirement to the packaging scripts.

(Is it possible to improve the Travis-CI configuration so things like packaging can be checked for errors prior to merge?)

@ffurano
Copy link
Contributor

ffurano commented Jun 14, 2018

Hi Brian, I am on it. Will know by tomorrow, I am off but will take a look anyway.

@ffurano ffurano merged commit 10765d5 into xrootd:master Jun 15, 2018
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.

None yet

4 participants