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

Enable GitHub-based remote dependencies #101

Merged
merged 9 commits into from Jul 2, 2021

Conversation

digorithm
Copy link
Member

@digorithm digorithm commented Jun 29, 2021

As described in #67

These changes enable users to specify dependencies that are
hosted on GitHub by defining the property git = ":url"
in the dependency, e.g:

std = { git = ":url"}

For now the GitHub repo must be public.
It also enables users to specify either a branch or a versioned tag
along with the git repo, e.g:

std = { git = ":url", branch = "my-branch"}

and:

std = { git = ":url", version = "0.0.1"}

As of now, version takes precedence over branch references.

These remote dependencies will be installed under ~/.forc/.

If no version/branch is specified, it will download the default
branch at the latest commit.

Future work includes:

  • A command to check if there are remote changes to the dependency
  • A command to download updates to the dependencies
  • I kept this as simple as I could, trying to not pull all the complexity around dependency management that you see in places like cargo. That said, once we have registries and more evolved non-local dependency needs, we'll need to refactor some things, e.g moving dependency-related code out of the forc_build.rs file and creating proper dependency-related abstractions.
  • Lockfile

These changes enable users to specify dependencies that are
hosted on GitHub by defining the property `git = ":url"`
in the dependency, e.g:
```
std = { git = ":url"}
```
For now the GitHub repo must be public.
It also enables users to specify either a branch or a versioned tag
along with the git repo, e.g:
```
std = { git = ":url", branch = "my-branch"}
```
and:
```
std = { git = ":url", version = "0.0.1"}
```
As of now, version takes precedence over branch references.

These remote dependencies will be installed under `~/forc/`.

If no version/branch is specified, it will download the default
branch at the latest commit.

Future work includes:
- A command to check if there are remote changes to the dependency
- A command to download updates to the dependencies
@digorithm digorithm changed the title Enable GitHub-based remote dependencies Enable GitHub-based remote dependencies (#67) Jun 29, 2021
@digorithm digorithm changed the title Enable GitHub-based remote dependencies (#67) Enable GitHub-based remote dependencies Jun 29, 2021
@digorithm digorithm linked an issue Jun 29, 2021 that may be closed by this pull request
4 tasks
@adlerjohn adlerjohn added the forc label Jun 29, 2021
forc/src/ops/forc_build.rs Outdated Show resolved Hide resolved
forc/src/ops/forc_build.rs Outdated Show resolved Hide resolved
forc/Cargo.toml Outdated Show resolved Hide resolved
forc/src/ops/forc_build.rs Show resolved Hide resolved
}
}

fn download_tarball(url: &str, out_dir: &str) -> Result<String> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do the error messages in this function include the URL/location/agent, etc.? If not, can they be added?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, will look into that!

@adlerjohn
Copy link
Contributor

The description currently says

These remote dependencies will be installed under ~/forc/

Should this be ~/.forc instead? The code seems to indicate it's ~/.forc.

@digorithm
Copy link
Member Author

The description currently says

These remote dependencies will be installed under ~/forc/

Should this be ~/.forc instead? The code seems to indicate it's ~/.forc.

Yup, exactly. Was a typo, thanks!

@digorithm digorithm marked this pull request as ready for review June 30, 2021 22:19
This was referenced Jun 30, 2021
@digorithm digorithm requested a review from sezna July 1, 2021 17:54
adlerjohn
adlerjohn previously approved these changes Jul 1, 2021
leviathanbeak
leviathanbeak previously approved these changes Jul 2, 2021
Copy link
Contributor

@leviathanbeak leviathanbeak left a comment

Choose a reason for hiding this comment

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

LGTM, but of course now that I merged my PR there are some conflicts but it should be easy to fix it

sezna
sezna previously approved these changes Jul 2, 2021
@digorithm digorithm dismissed stale reviews from sezna, leviathanbeak, and adlerjohn via 82b6c98 July 2, 2021 16:53
Copy link
Contributor

@leviathanbeak leviathanbeak left a comment

Choose a reason for hiding this comment

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

👍

@digorithm digorithm merged commit ec3c96e into master Jul 2, 2021
@digorithm digorithm deleted the rodrigo/remote-dependencies branch July 2, 2021 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Install dependencies from GitHub
4 participants