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

feat(nargo)!: Require package type be specified in Nargo.toml #2134

Merged
merged 12 commits into from
Aug 4, 2023
Merged

feat(nargo)!: Require package type be specified in Nargo.toml #2134

merged 12 commits into from
Aug 4, 2023

Conversation

phated
Copy link
Contributor

@phated phated commented Aug 2, 2023

Description

Problem*

Resolves #2073

Summary*

This adds the type field to the Nargo.toml file, which is required. We use this to determine what the default entry point file should be but it also opens up the ability to allow custom paths (as per #2076) because we'll know what package type is represented by the custom path.

As a matter of cleanup and separation of concerns, this also moves the CrateType into Nargo as PackageType and adds some helpful error messages if a project is configured incorrectly (such as a missing main function when running nargo check against a binary package). These changes enable us to add a contracts package type for #2075

Since we want to generate library and binary crates, this adds --lib and --bin to the nargo new and nargo init commands to generate an appropriate project.

Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.

The required type field needs to be documented when we document the Nargo.toml file and we need to add the new flags for new and init commands.

Additional Context

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@github-actions github-actions bot mentioned this pull request Aug 2, 2023
Closed
TomAFrench
TomAFrench reviewed Aug 3, 2023
View reviewed changes
crates/nargo_cli/src/cli/init_cmd.rs Outdated Show resolved Hide resolved
crates/nargo_cli/src/cli/init_cmd.rs Outdated Show resolved Hide resolved
crates/nargo_cli/src/cli/init_cmd.rs Outdated Show resolved Hide resolved
crates/nargo/src/package.rs Outdated Show resolved Hide resolved
crates/fm/src/lib.rs Outdated Show resolved Hide resolved
crates/nargo/src/package.rs Outdated Show resolved Hide resolved
crates/nargo_cli/src/cli/init_cmd.rs Outdated Show resolved Hide resolved
crates/nargo_cli/src/manifest.rs Show resolved Hide resolved
crates/noirc_driver/src/lib.rs Outdated Show resolved Hide resolved
crates/noirc_frontend/src/hir/mod.rs Outdated Show resolved Hide resolved
This was referenced Aug 3, 2023
Closed
Open
@phated
Copy link
Contributor Author

phated commented Aug 3, 2023

This has been rebased on #2157 to show the usage of the trait.

TomAFrench
TomAFrench previously approved these changes Aug 4, 2023
View reviewed changes
crates/nargo_cli/src/errors.rs Outdated Show resolved Hide resolved
@TomAFrench TomAFrench added this pull request to the merge queue Aug 4, 2023
Merged via the queue into master with commit 1c991d0 Aug 4, 2023
4 checks passed
@TomAFrench TomAFrench deleted the phated/nargo-crate-type branch August 4, 2023 17:34
@kevaundray kevaundray mentioned this pull request Aug 4, 2023
Merged
TomAFrench added a commit that referenced this pull request Aug 7, 2023
@TomAFrench
Merge branch 'master' into jb/noir-testing-workflow
1b7eca9 
* master:
  chore: Remove symlink and dummy config file (#2200)
  fix: Fix an ICE when reassigning a mutable lambda variable to one with a different environment type (#2172)
  feat: Only create new witnesses for distinctiveness when duplicates exist (#2191)
  chore: Use helper functions for getting values of `AcirVar`s (#2194)
  feat: Add support for slices of structs and nested slices in brillig (#2084)
  feat: Perform sorting of constant arrays at compile time (#2195)
  chore: Improve unary error (#2199)
  chore: separate integration test cases into directories based on expected result (#2198)
  chore: remove stale comment (#2197)
  feat(nargo): Support custom entry points specified in TOML (#2158)
  fix(nargo): Indicate which TOML file is missing package name (#2177)
  fix: remove duplicated `name` option in `nargo new` (#2183)
  chore: add documentation to the `nargo lsp` command (#2169)
  feat(nargo)!: Require package `type` be specified in Nargo.toml (#2134)
  fix(nargo): Make dependencies section optional in TOML (#2161)
  chore: Do not create new memory block when not needed (#2142)
  fix: fix an ICE happening when we call a closure result from if/else (#2146)
  chore: remove unnecessary cloning of package dependencies (#2175)
TomAFrench added a commit that referenced this pull request Aug 7, 2023
@TomAFrench
Merge branch 'master' into assert-eq-builtin
8bd4cbd 
* master: (35 commits)
  feat: Issue warning for signed integers (#2185)
  chore: Add `noir_wasm` testing workflow (#1921)
  chore: Remove symlink and dummy config file (#2200)
  fix: Fix an ICE when reassigning a mutable lambda variable to one with a different environment type (#2172)
  feat: Only create new witnesses for distinctiveness when duplicates exist (#2191)
  chore: Use helper functions for getting values of `AcirVar`s (#2194)
  feat: Add support for slices of structs and nested slices in brillig (#2084)
  feat: Perform sorting of constant arrays at compile time (#2195)
  chore: Improve unary error (#2199)
  chore: separate integration test cases into directories based on expected result (#2198)
  chore: remove stale comment (#2197)
  feat(nargo): Support custom entry points specified in TOML (#2158)
  fix(nargo): Indicate which TOML file is missing package name (#2177)
  fix: remove duplicated `name` option in `nargo new` (#2183)
  chore: add documentation to the `nargo lsp` command (#2169)
  feat(nargo)!: Require package `type` be specified in Nargo.toml (#2134)
  fix(nargo): Make dependencies section optional in TOML (#2161)
  chore: Do not create new memory block when not needed (#2142)
  fix: fix an ICE happening when we call a closure result from if/else (#2146)
  chore: remove unnecessary cloning of package dependencies (#2175)
  ...
cruzdanilo added a commit to cruzdanilo/noir-merkle-root that referenced this pull request Oct 3, 2023
@cruzdanilo
🔧 package: add type, required by noir v10
e67343f 
noir-lang/noir#2134
cruzdanilo added a commit to cruzdanilo/noir-merkle-root that referenced this pull request Oct 3, 2023
@cruzdanilo
🔧 package: add type, required by nargo v10
7dc0951 
noir-lang/noir#2134
@cruzdanilo cruzdanilo mentioned this pull request Oct 3, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kobyhallx kobyhallx kobyhallx left review comments

@TomAFrench TomAFrench TomAFrench approved these changes

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

Successfully merging this pull request may close these issues.

CrateType should be a nargo concept
3 participants
@phated @TomAFrench @kobyhallx