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

Enable other programs to include headers using <> #54

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Enable other programs to include headers using <> #54

wants to merge 1 commit into from

Conversation

mapx
Copy link

@mapx mapx commented Oct 2, 2017

This modification enable other projects to include this library headers with '#include <>'.

@mapx
Copy link
Author

mapx commented Oct 4, 2017

There was a reverted pull request #31 doing the same thing, and related discussion #25 .
So this PR might of little help. I wish there would be some improvement, in one kind or another.

@floitsch
Copy link
Collaborator

floitsch commented Oct 8, 2017

Yeah...
Not sure what to do there. This is not something I'm very familiar with, and @ulfjack last time seemed to think that not having "." is the correct thing to do.

@EdSchouten
Copy link

This patch currently seems necessary for me to be able to build a piece of software that uses this library. I can change the includes in my codebase to use "double-conversion/*", but the problem is that this library also contains includes to its own header files of the form <double-conversion/*>. Without this patch, those includes don't resolve.

@ulfjack
Copy link

ulfjack commented Mar 25, 2019

In Bazel 0.22.0, the following code works for me:

WORKSPACE:

load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
git_repository(
  name = "double_conversion",
  remote = "https://github.com/google/double-conversion.git",
  branch = "master",
)

double/BUILD:

cc_test(
  name = "double",
  srcs = ["double.cc"],
  deps = ["@double_conversion//:double-conversion"],
)

double/double.cc:

#include <stdio.h>
#include "double-conversion/double-conversion.h"
int main(int argc, char** argv) {
  return 0;
}

As far as I can tell from the generated gcc command line, it looks like the current directory is already added to the quote includes. Can you point me at a case that doesn't work yet?

@floitsch
Copy link
Collaborator

@ulfjack : thanks for taking a look. I hope @EdSchouten or @mapx can find the time to respond to you.

Just as a side-note: just a few days ago I changed the code to use relative imports inside the project (except for the tests). Maybe that makes things simpler.

@EdSchouten
Copy link

Hi there!

Just as a side-note: just a few days ago I changed the code to use relative imports inside the project (except for the tests). Maybe that makes things simpler.

Yep. This fixes things for me. The problem was that the codebase itself contained #include <double-conversion/*.h> includes. Those are no longer present. Thanks!

@floitsch
Copy link
Collaborator

floitsch commented Sep 9, 2020

@belbel102711 : you approved this PR.
Does this mean that it is still necessary?

My interpretation was that @EdSchouten and @ulfjack both found that the PR is not necessary.

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.

6 participants