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

Improve scripts/add-revions.sh #332

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Improve scripts/add-revions.sh #332

wants to merge 2 commits into from

Conversation

hamishmack
Copy link
Contributor

The need to pass a BUILT_REPO arg is very confusing and it is small enough to just DL the latest to a temp dir.

This change also only uses the BUILT_REPO if there is no existing revision. If there is a revision it copies the .cabal file from there (in case it is not in the BUILT_REPO yet).

Also uses date -u instead of date --utc so that we don't have to install gdate on macOS to make it work.

@hamishmack hamishmack requested a review from a team as a code owner June 7, 2023 01:14
@hamishmack hamishmack self-assigned this Jun 7, 2023
@hamishmack hamishmack requested a review from a team as a code owner June 7, 2023 01:30
@hamishmack
Copy link
Contributor Author

If we need to pass in a different _repo we could make something like this work:

BUILT_REPO=_repo .scripts/add-revions.sh foo 1.0.0

usage
exit 1
fi

BUILT_REPO=$(mktemp -d)
mkdir -p $BUILT_REPO/index
curl -L https://input-output-hk.github.io/cardano-haskell-packages/01-index.tar.gz | tar -C $BUILT_REPO/index -xz
Copy link
Contributor

@andreabedini andreabedini Jun 7, 2023

Choose a reason for hiding this comment

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

One can do

curl -L https://input-output-hk.github.io/cardano-haskell-packages/01-index.tar.gz | tar xvz cardano-cli/8.0.0/cardano-cli.cabal
``` to get the latest revision for `cardano-cli-8.0.0`.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or even

curl -L https://input-output-hk.github.io/cardano-haskell-packages/package/cardano-cli-8.0.0/cardano-cli.cabal

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this even correct? This downloads only the index. I guess that probably works in this case but I'm not sure. I think it would be safer to get the entire latest repository using one of the methods in https://github.com/input-output-hk/cardano-haskell-packages/pull/323

andreabedini
andreabedini previously approved these changes Jun 7, 2023
Copy link
Contributor

@andreabedini andreabedini left a comment

Choose a reason for hiding this comment

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

LGTM

@michaelpj
Copy link
Contributor

This won't work now if you need to add a revision to something that's not in the remote, or if you're doing multiple revisions to something. That's uncommon, but kind of important when you need it. We could wrap this in something that fetches the repository, it would be nice to keep the bare one also.

@andreabedini andreabedini dismissed their stale review June 13, 2023 01:51

Michael's comments

The need to pass a `BUILT_REPO` arg is very confusing and it is small enough to just DL the latest to a temp dir.

This change also only uses the `BUILT_REPO` if there is no existing revision.  If there is a revision it copies the `.cabal` file from there (in case it is not in the BUILT_REPO yet).

Also uses `date -u` instead of `date --utc` so that we don't have to install `gdate` on macOS to make it work.
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