Skip to content

Crate keyword#303

Merged
KyrylR merged 8 commits intoBlockstreamResearch:masterfrom
Sdoba16:feature/crate-keyword
Apr 30, 2026
Merged

Crate keyword#303
KyrylR merged 8 commits intoBlockstreamResearch:masterfrom
Sdoba16:feature/crate-keyword

Conversation

@Sdoba16
Copy link
Copy Markdown
Collaborator

@Sdoba16 Sdoba16 commented Apr 27, 2026

Added support of crate keyword for local dependencies.
Added check that local files are dependent only with crate keyword.
Small fix for test_utils.
Closes #294

@Sdoba16 Sdoba16 requested a review from LesterEvSe April 27, 2026 13:44
@Sdoba16 Sdoba16 self-assigned this Apr 27, 2026
@LesterEvSe LesterEvSe added the enhancement New feature or request label Apr 27, 2026
@Sdoba16 Sdoba16 force-pushed the feature/crate-keyword branch 2 times, most recently from 6139038 to c5f33a7 Compare April 27, 2026 14:17
@LesterEvSe LesterEvSe marked this pull request as ready for review April 27, 2026 14:30
@LesterEvSe LesterEvSe requested a review from delta1 as a code owner April 27, 2026 14:30
@apoelstra
Copy link
Copy Markdown
Contributor

In c5f33a7:

This is full of unrelated lockfile changes.

Copy link
Copy Markdown
Collaborator

@LesterEvSe LesterEvSe left a comment

Choose a reason for hiding this comment

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

Update the section regarding the crate keyword in the doc/architecture.md file.

Comment thread src/test_utils.rs
Comment thread src/resolution.rs Outdated
Comment thread src/parse.rs Outdated
@Sdoba16 Sdoba16 force-pushed the feature/crate-keyword branch 2 times, most recently from e8d0f3d to fcdc6dd Compare April 28, 2026 09:31
@Sdoba16 Sdoba16 force-pushed the feature/crate-keyword branch from fcdc6dd to 94bd65c Compare April 28, 2026 10:38
@LesterEvSe LesterEvSe requested a review from KyrylR April 28, 2026 10:56
Comment thread src/parse.rs Outdated
Comment on lines 1464 to 1466
// Parse the base path prefix (e.g., `dependency_root_path::file::` or `dependency_root_path::dir::file::`).
// We require at least 2 segments here because a valid import needs a minimum
// of 3 items total: the dependency_root_path, the file, and the specific item/function.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we mention crate keyword here?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, I think so

@KyrylR
Copy link
Copy Markdown
Collaborator

KyrylR commented Apr 29, 2026

@Sdoba16 Sdoba16 force-pushed the feature/crate-keyword branch from 37ee79c to 43e1744 Compare April 29, 2026 14:06
@Sdoba16
Copy link
Copy Markdown
Collaborator Author

Sdoba16 commented Apr 29, 2026

Running https://github.com/Sdoba16/SimplicityHL/tree/feature/crate-keyword/functional-tests/valid-test-cases/external-library-uses-crate using simc directly fails in my case

Is this expected?

Should work with the updates

@KyrylR
Copy link
Copy Markdown
Collaborator

KyrylR commented Apr 29, 2026

Running Sdoba16/SimplicityHL@feature/crate-keyword/functional-tests/valid-test-cases/external-library-uses-crate using simc directly fails in my case
Is this expected?

Should work with the updates

Could we add a regression test for this? Something like below

use std::path::{Path, PathBuf};
use std::process::Command;

fn repo_path(path: &str) -> PathBuf {
    Path::new(env!("CARGO_MANIFEST_DIR")).join(path)
}

#[test]
fn cli_dependency_can_use_crate_root() {
    let root = repo_path("functional-tests/valid-test-cases/external-library-uses-crate");
    let main = root.join("main.simf");
    let ext_lib = root.join("ext_lib");
    let dep_arg = format!("{}:ext_lib={}", root.display(), ext_lib.display());

    let output = Command::new(env!("CARGO_BIN_EXE_simc"))
        .arg(main)
        .arg("--dep")
        .arg(dep_arg)
        .output()
        .expect("failed to run simc");

    assert!(
        output.status.success(),
        "simc failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}",
        output.status.code(),
        String::from_utf8_lossy(&output.stdout),
        String::from_utf8_lossy(&output.stderr),
    );
}

@Sdoba16 Sdoba16 force-pushed the feature/crate-keyword branch from f4b6a88 to 8f002f3 Compare April 30, 2026 09:07
Copy link
Copy Markdown
Collaborator

@LesterEvSe LesterEvSe left a comment

Choose a reason for hiding this comment

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

ACK 8f002f3; tested locally

Copy link
Copy Markdown
Collaborator

@KyrylR KyrylR left a comment

Choose a reason for hiding this comment

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

ACK 8f002f3; code review, successfully ran tests locally

@KyrylR KyrylR merged commit 3eed7a6 into BlockstreamResearch:master Apr 30, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Introduce crate keyword for local imports

4 participants