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.

Sign up for GitHub

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

Jump to bottom

Return all include dirs as absolute paths #49

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

Return all include dirs as absolute paths #49

wants to merge 1 commit into from

Conversation

filmor
Copy link

@filmor filmor commented Dec 8, 2023

No description provided.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 8, 2023
@michalmuskala
Copy link
Member

michalmuskala commented Dec 8, 2023 •
edited
Loading

Hi @filmor, thank you for the contribution.
Could you explain what problem is this change solving?

@filmor
Copy link
Author

filmor commented Dec 8, 2023

Eqwalizer (via ELP) doesn't handle -include("some_header.hrl"). correctly in dependencies of rebar3 projects. I've seen this for amqp_client, gproc and ecron, error like this:
image
Structure here: https://github.com/rabbitmq/rabbitmq-server/tree/main/deps/amqp_client

As you can see, amqp_client_internal.hrl is in include.

Without this patch, the path is not added to the include_dirs build info in Rust as the Rust code (rightly) expects absolute paths.

The actual problem is not completely solved by this (I think for rebar3 projects you are not filling include_path from include_dirs, not sure what the distinction is here), but it's a required change to fix elp.

@filmor filmor mentioned this pull request Dec 8, 2023
Closed
@ilya-klyuchnikov
Copy link
Member

@filmor - can you share step-by-step instructions to reproduce the behaviour? thanks

@filmor
Copy link
Author

filmor commented Dec 12, 2023

I'll try to reproduce the issue, but so far I've only seen the issue on our main codebase. The two submitted patches fix things for me, though.

facebook-github-bot pushed a commit to WhatsApp/erlang-language-platform that referenced this pull request Feb 16, 2024
@filmor @facebook-github-bot
Set both include_dirs and include_path for rebar3 deps (#13)
b33b594 
Summary:
This is a follow up of WhatsApp/eqwalizer#49 to set both the `include_dirs` and `include_path` entries for apps that are created from `rebar3` build-info.

Explanation: WhatsApp/eqwalizer#49 (comment)

Pull Request resolved: #13

Reviewed By: alanz

Differential Revision: D53850658

Pulled By: robertoaloi

fbshipit-source-id: 90e8da4258b69f64f487964fc0cde7ad910e5518
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@filmor @michalmuskala @ilya-klyuchnikov @facebook-github-bot