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

Update Store derives #547

Merged
merged 1 commit into from
Jan 11, 2023
Merged

Update Store derives #547

merged 1 commit into from
Jan 11, 2023

Conversation

schneems
Copy link
Contributor

Store cannot be cloned

I want to have write some code like this:

        let store = context.store.get_or_insert_with(|| {
            let mut rng = rand::thread_rng();
            let value = (0..64)
                .map(|_| rng.sample(rand::distributions::Alphanumeric) as char)
                .collect::<String>();

            let mut metadata = toml::value::Table::default();
            metadata.insert(String::from("SECRET_KEY_BASE"), value.into());

            Store { metadata }
        });

But I can't since the store isn't mutable and I cannot clone it:


   Compiling heroku-ruby-buildpack v0.0.0 (/Users/rschneeman/Documents/projects/work/buildpacks/buildpacks-ruby/buildpacks/ruby)
warning: unused variable: `store`
   --> buildpacks/ruby/src/main.rs:117:13
    |
117 |         let store = context.store.get_or_insert_with(|| {
    |             ^^^^^ help: if this is intentional, prefix it with an underscore: `_store`
    |
    = note: `#[warn(unused_variables)]` on by default

error[E0596]: cannot borrow `context.store` as mutable, as `context` is not declared as mutable
   --> buildpacks/ruby/src/main.rs:117:21
    |
65  |       fn build(&self, context: BuildContext<Self>) -> libcnb::Result<BuildResult, Self::Error> {
    |                       ------- help: consider changing this to be mutable: `mut context`
...
117 |           let store = context.store.get_or_insert_with(|| {
    |  _____________________^
118 | |             let mut rng = rand::thread_rng();
119 | |             let value = (0..64)
120 | |                 .map(|_| rng.sample(rand::distributions::Alphanumeric) as char)
...   |
126 | |             Store { metadata }
127 | |         });
    | |__________^ cannot borrow as mutable

For more information about this error, try `rustc --explain E0596`.
warning: `heroku-ruby-buildpack` (bin "heroku-ruby-buildpack") generated 1 warning (1 duplicate)
error: could not compile `heroku-ruby-buildpack` due to previous error; 1 warning emitted
warning: build failed, waiting for other jobs to finish...
warning: `heroku-ruby-buildpack` (bin "heroku-ruby-buildpack" test) generated 1 warning
error: could not compile `heroku-ruby-buildpack` due to previous error; 1 warning emitted
[Finished running. Exit status: 101]

I'm able to extract the metadata and work on that directly, but I would rather maintain and manipulate Store directly.

        let mut metadata = match context.store {
            Some(store) => store.metadata,
            None => toml::value::Table::default(),
        }
        .clone();

        metadata.entry("SECRET_KEY_BASE").or_insert_with(|| {
            let mut rng = rand::thread_rng();

            (0..64)
                .map(|_| rng.sample(rand::distributions::Alphanumeric) as char)
                .collect::<String>()
                .into()
        });

Store has no default

Not a big deal if it's not there, but Option has a get_or_default in nightly and it would be neat to be able to context.store.get_or_default when that is stable.

@schneems schneems requested a review from a team as a code owner January 10, 2023 16:05
@schneems schneems force-pushed the schneems/clone-store branch from 10aecc6 to 318cc3e Compare January 10, 2023 16:45
CHANGELOG.md Outdated Show resolved Hide resolved
@Malax
Copy link
Member

Malax commented Jan 11, 2023

Option has a get_or_default in nightly

@schneems FWIW, there is also unwrap_or_default available today. :)

## Store cannot be cloned 

I want to have write some code like this:

```rust
        let store = context.store.get_or_insert_with(|| {
            let mut rng = rand::thread_rng();
            let value = (0..64)
                .map(|_| rng.sample(rand::distributions::Alphanumeric) as char)
                .collect::<String>();

            let mut metadata = toml::value::Table::default();
            metadata.insert(String::from("SECRET_KEY_BASE"), value.into());

            Store { metadata }
        });
```

But I can't since the store isn't mutable and I cannot clone it:

```

   Compiling heroku-ruby-buildpack v0.0.0 (/Users/rschneeman/Documents/projects/work/buildpacks/buildpacks-ruby/buildpacks/ruby)
warning: unused variable: `store`
   --> buildpacks/ruby/src/main.rs:117:13
    |
117 |         let store = context.store.get_or_insert_with(|| {
    |             ^^^^^ help: if this is intentional, prefix it with an underscore: `_store`
    |
    = note: `#[warn(unused_variables)]` on by default

error[E0596]: cannot borrow `context.store` as mutable, as `context` is not declared as mutable
   --> buildpacks/ruby/src/main.rs:117:21
    |
65  |       fn build(&self, context: BuildContext<Self>) -> libcnb::Result<BuildResult, Self::Error> {
    |                       ------- help: consider changing this to be mutable: `mut context`
...
117 |           let store = context.store.get_or_insert_with(|| {
    |  _____________________^
118 | |             let mut rng = rand::thread_rng();
119 | |             let value = (0..64)
120 | |                 .map(|_| rng.sample(rand::distributions::Alphanumeric) as char)
...   |
126 | |             Store { metadata }
127 | |         });
    | |__________^ cannot borrow as mutable

For more information about this error, try `rustc --explain E0596`.
warning: `heroku-ruby-buildpack` (bin "heroku-ruby-buildpack") generated 1 warning (1 duplicate)
error: could not compile `heroku-ruby-buildpack` due to previous error; 1 warning emitted
warning: build failed, waiting for other jobs to finish...
warning: `heroku-ruby-buildpack` (bin "heroku-ruby-buildpack" test) generated 1 warning
error: could not compile `heroku-ruby-buildpack` due to previous error; 1 warning emitted
[Finished running. Exit status: 101]
```

I'm able to extract the metadata and work on that directly, but I would rather maintain and manipulate Store directly.

```rust
        let mut metadata = match context.store {
            Some(store) => store.metadata,
            None => toml::value::Table::default(),
        }
        .clone();

        metadata.entry("SECRET_KEY_BASE").or_insert_with(|| {
            let mut rng = rand::thread_rng();

            (0..64)
                .map(|_| rng.sample(rand::distributions::Alphanumeric) as char)
                .collect::<String>()
                .into()
        });
```


## Store has no default

Not a big deal if it's not there, but Option has a `get_or_default` in nightly and it would be neat to be able to `context.store.get_or_default` when that is stable.
@schneems schneems force-pushed the schneems/clone-store branch from 318cc3e to 0eb8f66 Compare January 11, 2023 14:46
@Malax Malax merged commit b45bd79 into main Jan 11, 2023
@Malax Malax deleted the schneems/clone-store branch January 11, 2023 16:06
schneems added a commit to heroku/buildpacks-ruby that referenced this pull request Jan 11, 2023
Using the new Clone and Default derives from heroku/libcnb.rs#547
@schneems
Copy link
Contributor Author

Thanks for the merge and release. Looks much better heroku/buildpacks-ruby@78a45b8. (Also ready for review)

schneems added a commit to heroku/buildpacks-ruby that referenced this pull request Jan 31, 2023
Using the new Clone and Default derives from heroku/libcnb.rs#547
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants