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

Upgrade rust dependencies #181

Merged
merged 13 commits into from
Mar 6, 2024
Merged

Upgrade rust dependencies #181

merged 13 commits into from
Mar 6, 2024

Conversation

aramikm
Copy link
Collaborator

@aramikm aramikm commented Mar 4, 2024 •
edited
Loading

Goal

The goal of this PR is to upgrade dependencies on the graph-sdk. To add support for more targets like aarch64 on linux, the CI was demanding newer Rust version and that was the reason to upgrade everything.

Closes #182

Changes

  • Upgraded Rust version to "1.76.0" stable
  • Upgraded all crates to latest stable versions
  • Fixed cargo deny toml to work with the latest updates (had to manually allow some licenses)
  • Bumped all internal packaged from 1.0.0 to 1.0.2. (Skipped 1.0.1 since wanted to keep the internal and published released versions in sync if possible and we already released 1.0.1)

Open issues

  • FFI tests is broken in CI and had to comment it temporarily

@aramikm aramikm changed the title upgrade rust dependencies Upgrade rust dependencies Mar 5, 2024
@aramikm aramikm marked this pull request as ready for review March 5, 2024 01:59
mattheworris
mattheworris reviewed Mar 5, 2024
View reviewed changes
Makefile Outdated Show resolved Hide resolved
JoeCap08055
JoeCap08055 reviewed Mar 5, 2024
View reviewed changes
bridge/node/Cargo.toml
once_cell = "1.18.0"
dsnp-graph-core = { version = "1.0.2", path = "../../core" }
dsnp-graph-config = { version = "1.0.2", path = "../../config" }
neon = { version = "1.0.0", default-features = false, features = ["napi-6"] }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, 1.0. Any significant enhancements to neon? Worth filing an issue in the backlog to review our usage?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have not looked into it yet. Feel free to take a look since you guys know more about it than me

JoeCap08055
JoeCap08055 reviewed Mar 5, 2024
View reviewed changes
Copy link
Collaborator

@JoeCap08055 JoeCap08055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just a little confused on versioning. I see we've jumped from 1.0.0 to 1.0.2, but in the java builds we have 1.0.1, and in one spot still *-SNAPSHOT. Can you clarify how they interrelate?

aramikm
aramikm commented Mar 5, 2024
View reviewed changes
.cargo-deny.toml
@@ -39,7 +39,7 @@ targets = [
# If true, metadata will be collected with `--all-features`. Note that this can't
# be toggled off if true, if you want to conditionally enable `--all-features` it
# is recommended to pass `--all-features` on the cmd line instead
all-features = true
# all-features = true
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were all deprecated. More info in EmbarkStudios/cargo-deny#611

.cargo-deny.toml Show resolved Hide resolved
.github/workflows/pr-verify.yml Show resolved Hide resolved
aramikm and others added 4 commits March 5, 2024 12:07
@aramikm
use components
40dd6ae
@aramikm
rev format
374c867
@aramikm
some more changes
05a930d
@saraswatpuneet
Ffi check (#183)
12608e9 
# Goal
The goal of this PR is <!-- insert goal here -->

Closes <!-- issue # -->

# Discussion
<!-- List discussion items -->

# Checklist
- [ ] Tests added
JoeCap08055
JoeCap08055 reviewed Mar 6, 2024
View reviewed changes
Copy link
Collaborator

@JoeCap08055 JoeCap08055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last question: when building the main libraries, we've bumped our version to 1.0.2 for all of the rust stuff, but when building the Java bridge, we build a jar labeled with 1.0.1 as the version. Why the discrepancy?

@aramikm
Copy link
Collaborator Author

aramikm commented Mar 6, 2024

@JoeCap08055 this was kinda described in the PR description but open to suggestions if you think we should do it differently

Bumped all internal packaged from 1.0.0 to 1.0.2. (Skipped 1.0.1 since wanted to keep the internal and published released versions in sync if possible and we already released 1.0.1)

saraswatpuneet
saraswatpuneet approved these changes Mar 6, 2024
View reviewed changes
Copy link
Collaborator

@saraswatpuneet saraswatpuneet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, can go once comments are resolved

@aramikm
Copy link
Collaborator Author

aramikm commented Mar 6, 2024

Merging this PR to get the ball rolling. Any existing concerns will be addressed in future PR's before cutting a new release.

@aramikm aramikm merged commit d28f428 into main Mar 6, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoeCap08055 JoeCap08055 JoeCap08055 left review comments

@mattheworris mattheworris mattheworris left review comments

@saraswatpuneet saraswatpuneet saraswatpuneet approved these changes

@wilwade wilwade Awaiting requested review from wilwade

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[GraphSDK] Update Rust and dependencies
5 participants
@aramikm @wilwade @mattheworris @saraswatpuneet @JoeCap08055