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

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

Closed
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
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 💖

Copy link
Contributor

@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
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build artefacts cannot be found after successful build
3 participants