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

Logic for handling git dependencies #82

Closed
wants to merge 1 commit into from

Conversation

adityakalia
Copy link

No description provided.

@snoyberg snoyberg mentioned this pull request Apr 21, 2022
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
Dockerfile Outdated
@@ -81,7 +81,7 @@ FROM rust:1.60.0-alpine as base-optimizer

# Being required for gcc linking
Copy link
Member

Choose a reason for hiding this comment

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

Let's add this comment since git-fetch-with-cli can be necessary in cases of public git dependencies too sometimes.

Suggested change
# Being required for gcc linking
# musl-dev is required for gcc linking.
# Adding Git for usage within projects that use `git-fetch-with-cli = true`.

@webmaster128
Copy link
Member

Reopened to trigger Circle CI checks

@@ -4,6 +4,7 @@
set -o errexit -o nounset -o pipefail
command -v shellcheck >/dev/null && shellcheck "$0"

[ -n "${GIT_CREDENTIALS=""}" ] && echo "$GIT_CREDENTIALS" >~/.git-credentials && git config --global credential.helper store
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[ -n "${GIT_CREDENTIALS=""}" ] && echo "$GIT_CREDENTIALS" >~/.git-credentials && git config --global credential.helper store
[ -n "${GIT_CREDENTIALS:=""}" ] && echo "$GIT_CREDENTIALS" >~/.git-credentials && git config --global credential.helper store

Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

I wonder if this feature is fundamentally conflicting with the goals of this repository. Once you support private dependencies, you give up reproducible building for anyone who does not have the credentials. When you don't care about reproducibility, you can as well use a build script like this:

# compile all contracts
for C in ./contracts/*/
do
  echo "Compiling $(basename "$C")..."
  (cd "$C" && RUSTFLAGS='-C link-arg=-s' cargo build --release --target wasm32-unknown-unknown --locked)
done

and run a local wasm-opt on the results.

Here is another project that shows how things get simpler and fancier if you are building a tool for a different use case: https://github.com/mandrean/cw-optimizoor

I'd like to avoid adding overhead here (in code, text and review capacity) that at the end of the day builds proprietary build systems. And we should clearly define goals and non-goals of this repo.

@webmaster128
Copy link
Member

Closing because private dependencies means you can't do publicly verifyable builds

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.

None yet

3 participants