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

Remove clang-sys dependency #238

Merged
merged 30 commits into from
May 10, 2024
Merged

Remove clang-sys dependency #238

merged 30 commits into from
May 10, 2024

Conversation

CGMossa
Copy link
Member

@CGMossa CGMossa commented May 9, 2024

This PR accomplishes three very distinct things:

  • Adds a missing : infront of cargo:: everywhere.
  • Adds metadata, which have been missing from build.rs forever
    Update: the above items are unnecessary. rust 1.77 have a new syntax for build.rs. I didn't
    catch that initially. This has now been reverted, and MSRV for this PR is the same.
  • Removes the inclusion of clang/clang-sys, by instead of the elaborate processing, we instead, ignore all items coming from headers that aren't the R-headers.

@yutannihilation
Copy link
Contributor

I didn't know the new syntax. Is it your intention to drop
the support on Rust <1.77?

Stick to cargo: only if the support of Rust version older than 1.77 is required.

https://doc.rust-lang.org/cargo/reference/build-scripts.html

@CGMossa
Copy link
Member Author

CGMossa commented May 10, 2024

Wow! Thanks! I did not know about syntax change in cargo 1.77. Yesterday, while I was trying to make this strategy work, I tried everything, and then I thought we had used the wrong syntax.

I've reverted this.

@Ilia-Kosenkov You asked about the Rcomplex thing. Since we are filtering elements in a new way, maybe now,
there is an element not being filtered, or something like that. But I just checked, and our definition of Rcomplex is still the same.
The items from wrapper.h are being considered just fine.

@CGMossa CGMossa requested a review from Ilia-Kosenkov May 10, 2024 09:26
@CGMossa
Copy link
Member Author

CGMossa commented May 10, 2024

So one explanation to the extra new items that somewhat looks like:

enum {SORTED_DECR_NA_1ST = -2,
      SORTED_DECR = -1,
      UNKNOWN_SORTEDNESS = INT_MIN, /*INT_MIN is NA_INTEGER! */
      SORTED_INCR = 1,
      SORTED_INCR_NA_1ST = 2,
      KNOWN_UNSORTED = 0};

these are items that stem from an anonymous C-enum. Before, with the clang-processing, we would just ignore it.
This time, it is being included. There is zero harm with it being present.

@CGMossa CGMossa mentioned this pull request May 10, 2024
2 tasks
@CGMossa CGMossa merged commit 542d266 into master May 10, 2024
@CGMossa CGMossa deleted the remove_clang branch May 10, 2024 20:34
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.

3 participants