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

TS-4560: Fix C++ API shared_ptr detection. #720

Merged
merged 1 commit into from
Jun 17, 2016
Merged

Conversation

jpeach
Copy link
Contributor

@jpeach jpeach commented Jun 16, 2016

Since ink_autoconf.h is not installed, we can't use it to figure
out which shared_ptr implementation to use. Further, Traffic Server
might not have been built with the same toolchain that 3rd party
plugins are using.

Since shared_ptr is used in the C++ API headers, both Traffic Server
and the plugins need to agree on which implementation to use. The
best (but not perfect) solution is to hardcode it into the header
file during the Traffic Server build. This means that both parties
are consistent and any remaining problems could be attributed to
C++ standard library compatibility.

@jpeach
Copy link
Contributor Author

jpeach commented Jun 16, 2016

Ping @bgaff @SolidWallOfCode

Since ink_autoconf.h is not installed, we can't use it to figure
out which shared_ptr implementation to use. Further, Traffic Server
might not have been built with the same toolchain that 3rd party
plugins are using.

Since shared_ptr is used in the C++ API headers, both Traffic Server
and the plugins need to agree on which implementation to use. The
best (but not perfect) solution is to hardcode it into the header
file during the Traffic Server build. This means that both parties
are consistent and any remaining problems could be attributed to
C++ standard library compatibility.
@atsci
Copy link

atsci commented Jun 17, 2016

FreeBSD build failed! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/295/ for details.

@atsci
Copy link

atsci commented Jun 17, 2016

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/187/ for details.

@zwoop zwoop added CPP API Build work related to build configuration or environment labels Jun 17, 2016
@zwoop zwoop added this to the 7.0.0 milestone Jun 17, 2016
@zwoop
Copy link
Contributor

zwoop commented Jun 17, 2016

Not sure why the FreeBSD build failed, trying again. [approve ci]

@atsci
Copy link

atsci commented Jun 17, 2016

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/296/ for details.

@atsci
Copy link

atsci commented Jun 17, 2016

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/188/ for details.

@jpeach
Copy link
Contributor Author

jpeach commented Jun 17, 2016

we should do the same with all the ink config stuff rather than including ink_config

I'm not sure what you mean by this. This patch is removing ink_autoconf.h from the public C++ API.

@bgaff
Copy link
Member

bgaff commented Jun 17, 2016

Sorry I was referring specifically to the config ( the txn config set enums ) but I think this solves that too.

Please land this, thanks for doing it!

@jpeach jpeach merged commit 480b899 into apache:master Jun 17, 2016
JosiahWI pushed a commit to JosiahWI/trafficserver that referenced this pull request Jul 19, 2023
…#8776) (apache#720)

* Fix incrementation and logging of server connection attempts for transaction.

* Changes to PR 8776 required by code review.

Co-authored-by: Walt Karas <wkaras@yahooinc.com>
(cherry picked from commit eb250b5)

Conflicts:
	proxy/http/HttpSM.cc
	proxy/http/HttpTransact.cc
	proxy/http/HttpTransact.h
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build work related to build configuration or environment CPP API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants