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

fix: set variable CARGO_TARGET_DIR to build contract in contract project #234

Closed
wants to merge 2 commits into from
Closed

fix: set variable CARGO_TARGET_DIR to build contract in contract project #234

wants to merge 2 commits into from

Conversation

AlexD10S
Copy link
Collaborator

@AlexD10S AlexD10S commented Jul 1, 2024

Closes #196

Fixes the bug reported above setting the env variable CARGO_TARGET_DIR before building the contract, as cargo-contract enforce the take the root_package to place the artifacts when building the contract. See here and here.

Added a unit test that ensures this variable is set.

@AlexD10S AlexD10S requested a review from al3mart July 1, 2024 18:40
al3mart
al3mart approved these changes Jul 1, 2024
View reviewed changes
Copy link
Contributor

@al3mart al3mart left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks 💖

brunopgalvao
brunopgalvao reviewed Jul 2, 2024
View reviewed changes
Copy link
Collaborator

@brunopgalvao brunopgalvao left a comment

Choose a reason for hiding this comment

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

Not sure I follow what @bolajahmad is saying. CARGO_TARGET_DIR should be used. In general, CARGO_TARGET_DIR is the location of where to place all generated artifacts, relative to the current working directory. See here.

IIU this PR correctly, the reason we want to set the CARGO_TARGET_DIR is because if the default is not specified then the directory named target located at the root of the workspace will be used. As stated here.

@AlexD10S
Copy link
Collaborator Author

AlexD10S commented Jul 2, 2024

Not sure I follow what @bolajahmad is saying. CARGO_TARGET_DIR should be used. In general, CARGO_TARGET_DIR is the location of where to place all generated artifacts, relative to the current working directory. See here.

IIU this PR correctly, the reason we want to set the CARGO_TARGET_DIR is because if the default is not specified then the directory named target located at the root of the workspace will be used. As stated here.

In the issue there is a discussion on how to fix it. But in this PR, you understood it correctly the fix is set the CARGO_TARGET_DIR variable to don't use the default one when generating the contract.
What is your concern? I'm open to considering other approaches.

@AlexD10S AlexD10S mentioned this pull request Jul 2, 2024
Open
@AlexD10S
Copy link
Collaborator Author

AlexD10S commented Jul 2, 2024

Closing it for now. Reason: #196 (comment)

@AlexD10S AlexD10S closed this Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brunopgalvao brunopgalvao brunopgalvao left review comments

@al3mart al3mart al3mart 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.

Build artefacts cannot be found after successful build
3 participants
@AlexD10S @brunopgalvao @al3mart