feat(service): get rid of `cargo` dependency by iamwacko · Pull Request #922 · shuttle-hq/shuttle

iamwacko · 2023-05-15T18:35:47Z

Description of change

is_next, is_alpha, ensure_bin, ensure_cdylib, and clean_crate no longer require cargo.

partially closes #756, particularly service part 1.

How Has This Been Tested (if applicable)?

I ran cargo test, cargo fmt, and cargo clippy.

…lean_crate to not use cargo

iamwacko · 2023-05-15T18:39:30Z

I have time, so I can do service part 2 as well.

iulianbarbu · 2023-05-16T16:35:42Z

I have time, so I can do service part 2 as well.

Sounds good @iamwacko ! Let us know if you need help. We will review this whenever it's ready.

…ore clear

jonaro00

It's hard to accurately review Rust code without rust-analyzer at hand, but here are some thoughts I had. Don't worry if I'm wrong about something.

service/src/builder.rs

iulianbarbu · 2023-05-19T13:25:06Z

@iamwacko let me know if you have questions on the changes I proposed and we can set up a call to get through them. The PR is on the right path and we'll just need to polish a bit the code async-wise, while keeping it on par with the old functionality.

We have a codebase walkthrough session next Monday (if that helps) and a mentoring session next Wednesday, but feel free to hit me up with qs in private on Discord if you feel stuck.

…st exists, and improve panic messages

jonaro00

I found some more small things.

I read some more about tokio's Command.
It sounds like it is the proper replacement for spawn_blocking + std Command, so that should be fine.

Cargo.toml

service/src/builder.rs

oddgrd

This looks pretty close @iamwacko! I left a few small comments, but I think the main thing we need to fix is sending the build logs to the build_workspace caller, which Iulian pointed out. Let us know if you are stuck on this and need any help!

Cargo.lock

service/src/builder.rs

oddgrd

Nice progress! I took a look at your latest changes and saw what may be an easier way to read the output, let me know if you think it could work!

service/Cargo.toml

service/src/builder.rs

iulianbarbu

Thanks @iamwacko ! This is great 🥳 .

iulianbarbu · 2023-06-05T20:43:29Z

I think there is a regression on shuttle-next. I am not able to run the next/hello-world because of:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: status: Unknown, message: "failed to read input file", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc", "date": "Mon, 05 Jun 2023 20:43:08 GMT", "content-length": "0"} }', /Users/iulianbarbu/repos/shuttle/cargo-shuttle/src/lib.rs:779:51

We must investigate the failing integration test on cargo-shuttle.

iamwacko · 2023-06-05T22:13:01Z

I see the problem. - gets turned into _ by cargo, but I didn't account for that so it tries to execute hello-world.wasm instead of hello_world.wasm. I also didn't catch it because for a while cargo-shuttle was failing for unrelated reasons.

iamwacko · 2023-06-06T03:21:08Z

Now the cargo-shuttle test is failing with thread 'init::non_interactive_basic_init' panicked at 'Could not query the latest version of shuttle-runtime', cargo-shuttle/src/init.rs:994:33. It works locally, so I have no idea what has gone wrong. I haven't even touched the code that would make this happen.

iamwacko · 2023-06-06T03:25:12Z

Looks like the only test failures I have are #821

iulianbarbu

Thanks @iamwacko for fixing the shuttle-next issue. Yes, it seems we're getting the error corresponding to #821 in CI. The tests pass locally for me as well.

iamwacko added 3 commits May 15, 2023 11:26

feat: reimplement is_next, is_alpha, ensure_bin, ensure_cdylib, and c…

ba211f3

…lean_crate to not use cargo

Merge branch 'shuttle-hq:main' into remove_cargo

ebb9c3b

fix: fmt

85a1106

iamwacko added 4 commits May 15, 2023 14:37

fix: clippy errors

a173d4b

fix: change an error to satisfy a test

0146dcb

fix: cargo is_lib and cargo_metadata is_lib are different

8c581db

chore: cargo fmt

18b805a

iulianbarbu added the Shuttle Batch label May 16, 2023

iulianbarbu added the M label May 16, 2023

iamwacko added 7 commits May 18, 2023 09:46

feat: add compiler function to replace cargo, removed cargo

eb0a469

fix: satisfied clippy, removed unused code

ea7566c

fix: current dir is located earlier

ee486de

fix: cwd moved back again

315097c

fix: use workspace root instead of current_directory()

f1fc9ab

fix: use project_directory instead of current_directory or root

50c5ac6

fix: more working directory

ce07665

jonaro00 mentioned this pull request May 18, 2023

[Enhancement]: Get rid of cargo dependency #756

Closed

iamwacko added 2 commits May 18, 2023 13:23

fix: working directory finally works

50b6a71

feat: 1 job failed panic when the Shuttle runtime is missing is now m…

89169e2

…ore clear

iulianbarbu added the A-service label May 19, 2023

jonaro00 reviewed May 19, 2023

View reviewed changes

service/src/builder.rs Outdated Show resolved Hide resolved

service/src/builder.rs Outdated Show resolved Hide resolved

service/src/builder.rs Outdated Show resolved Hide resolved

service/src/builder.rs Show resolved Hide resolved

iulianbarbu suggested changes May 19, 2023

View reviewed changes

iamwacko added 5 commits May 19, 2023 09:46

fix: use oneshot instead of pipe, remove debug print, check if manife…

726156b

…st exists, and improve panic messages

fix: add .await

6bbe48d

fix: remove unecessary clone

de2f506

docs: change comment

4993d25

Merge branch 'main' into remove_cargo

087315f

jonaro00 requested changes May 31, 2023

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

service/src/builder.rs Outdated Show resolved Hide resolved

oddgrd reviewed May 31, 2023

View reviewed changes

Cargo.lock Show resolved Hide resolved

service/src/builder.rs Show resolved Hide resolved

service/src/builder.rs Outdated Show resolved Hide resolved

iamwacko added 6 commits May 31, 2023 11:57

feat: deployment boolean added

8a2e116

fix: moved process to service

3863f8c

feat: add stderr and stdout output

7365e01

feat: refactored the log outputting to be a bit nicer

516b2fc

fix: cargo fmt

9f23c11

fix: stop errors

e85fd99

oddgrd reviewed Jun 4, 2023

View reviewed changes

service/Cargo.toml Outdated Show resolved Hide resolved

service/src/builder.rs Show resolved Hide resolved

iamwacko added 3 commits June 4, 2023 12:09

Merge branch 'main' into remove_cargo

fb13456

feat: use BufReader instead of weirdness

bef9e17

feat: remove pipe

acf9ba6

iamwacko marked this pull request as ready for review June 4, 2023 20:08

iamwacko requested a review from iulianbarbu June 4, 2023 20:09

Merge branch 'main' into remove_cargo

b556eeb

iulianbarbu previously approved these changes Jun 5, 2023

View reviewed changes

iamwacko added 5 commits June 5, 2023 15:23

fix: replace - with _ for files

cdfacfa

fix: replace string with char

84cea55

fix: clippy single_char_pattern

2107827

fix: underscore thing only happens to libs

7b47000

fix: trigger ci, it works locally

03a297d

iulianbarbu approved these changes Jun 6, 2023

View reviewed changes

iulianbarbu merged commit 7c01a73 into shuttle-hq:main Jun 6, 2023

iulianbarbu mentioned this pull request Jun 6, 2023

feat: use cargo generate instead of hardcoding examples source code #888

Merged

jonaro00 mentioned this pull request Jun 13, 2023

Allow using a different linker in deployment #1000

Open

1 task

Conversation

iamwacko commented May 15, 2023

Description of change

How Has This Been Tested (if applicable)?

Uh oh!

iamwacko commented May 15, 2023

Uh oh!

iulianbarbu commented May 16, 2023

Uh oh!

jonaro00 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iulianbarbu commented May 19, 2023

Uh oh!

jonaro00 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

oddgrd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oddgrd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

iulianbarbu left a comment

Choose a reason for hiding this comment

Uh oh!

iulianbarbu commented Jun 5, 2023

Uh oh!

iamwacko commented Jun 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iamwacko commented Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iamwacko commented Jun 6, 2023

Uh oh!

iulianbarbu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

iamwacko commented Jun 5, 2023 •

edited

Loading

iamwacko commented Jun 6, 2023 •

edited

Loading

iulianbarbu left a comment •

edited

Loading