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: version inconsistencies when using custom version #62

Closed
wants to merge 1 commit into from
Closed

fix: version inconsistencies when using custom version #62

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 11, 2023

Problem
Recently, we initiated the use of a custom version in the workspace-rs crate to fix one of the issue nearcore had when building wasms with newer rust version. However, this change introduced an unintended consequence where other crates began encountering issues due to a mismatch in the sandbox version being utilized. The sandbox was defaulting to downloading this version https://github.com/near/sandbox/blob/ee06569991da4a3a69f3cc575715a3fdc06935da/crate/src/lib.rs#L15 instead of the newly specified custom version. The root cause of this mismatch seems to be originating from line https://github.com/near/sandbox/blob/ee06569991da4a3a69f3cc575715a3fdc06935da/crate/src/lib.rs#L126


Solution
This Pull Request proposes a solution to harmonize the versioning system by recording the SANDBOX_VERSION in a temporary file. This approach ensures that the version data can be retrieved and utilized consistently across different crates, thereby preventing the sandbox from defaulting to the original version. The temp file acts as a single source of truth for the version information and is referred by :https://github.com/near/sandbox/blob/ee06569991da4a3a69f3cc575715a3fdc06935da/crate/src/lib.rs#L123

to determine the correct version to download and use.

@frol
Copy link
Collaborator

frol commented Sep 11, 2023

@shariffdev I don't understand the root cause of the problem. Can you elaborate more? The proposed solution is quite a hack, so I would like to understand better the reasoning for it

@ghost
Copy link
Author

ghost commented Sep 12, 2023

@shariffdev I don't understand the root cause of the problem. Can you elaborate more? The proposed solution is quite a hack, so I would like to understand better the reasoning for it

@frol If you follow this issue in workspace-rs, it will make a little bit of sense, but in general, if a dependency is using the workspace-rs crate to run a sandbox, it will not use the version specified in the workspace-rs crate, but it defaults to downloading the default commit hash which is found in the sandbox.

Yeah, I agree this is a hack but I didn't know any other method I can use, any suggestions are welcome, I tried using environment variables to configure the versioning to realize the variables set in rust doesn't persist across different processes.

@iho
Copy link
Contributor

iho commented Sep 24, 2023

Hi @shariffdev
Can you please elaborate on the chain of commands that we need to call here? How did you set env variables?
You can set your variables when you are spawning a new process:
https://github.com/near/sandbox/blob/main/crate/src/sync.rs#L8

@frol
Copy link
Collaborator

frol commented Sep 25, 2023

I believe it was superseded by #63, so I am closing this PR.

@frol frol closed this Sep 25, 2023
@ghost ghost deleted the fix/version-installation branch October 3, 2023 16:02
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.

3 participants