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

Support private github repository #1690

Merged
merged 58 commits into from
Jun 10, 2024
Merged

Support private github repository #1690

merged 58 commits into from
Jun 10, 2024

Conversation

NobodyXu
Copy link
Member

@NobodyXu NobodyXu commented May 5, 2024

Fixed #1681

@NobodyXu NobodyXu force-pushed the feat/private-github-repo branch from c0bca8f to 77d0e54 Compare May 5, 2024 13:46
@NobodyXu NobodyXu force-pushed the feat/private-github-repo branch from 77d0e54 to 9efe395 Compare May 5, 2024 13:49
@NobodyXu NobodyXu force-pushed the feat/private-github-repo branch from 74bfc87 to d2914cc Compare May 6, 2024 12:47
@NobodyXu NobodyXu force-pushed the feat/private-github-repo branch from 555b101 to 6c67c7d Compare May 7, 2024 12:44
@scullionw
Copy link

is there anything we can do to help move this forward? maybe testing a build?

@NobodyXu
Copy link
Member Author

is there anything we can do to help move this forward? maybe testing a build?

I just need to spend more time on this, I'm a bit busy recently, but rest asure, I didn't forget it.

@NobodyXu
Copy link
Member Author

Update: I'm a bit busy this week.

cc @scullionw we actually need more contributors for cargo-binstall.

If you are willing to submit a PR, then I can do a code review very quickly.

If you have time, you could pick up where I have left and I'm willing to answer any questions you have related to the codebase.

NobodyXu added 9 commits May 25, 2024 11:00
To make it easier to create generic function

Signed-off-by: Jiahao XU <[email protected]>
Signed-off-by: Jiahao XU <[email protected]>
`cargo test` run all tests in one process.

As such, `set_global_default` would fail on the second call.

Signed-off-by: Jiahao XU <[email protected]>
which is always set to `None`

Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu NobodyXu marked this pull request as ready for review June 9, 2024 14:48
@NobodyXu NobodyXu requested a review from passcod June 9, 2024 14:59
@NobodyXu
Copy link
Member Author

cc @scullionw This PR is now ready for review, you should be able to download from private repositories using this PR, could you have a try please?

@NobodyXu NobodyXu added this pull request to the merge queue Jun 10, 2024
@NobodyXu NobodyXu removed this pull request from the merge queue due to a manual request Jun 10, 2024
Copy link
Member Author

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

It seems that we have hit the gh token limit for CI, since many tests are run under it.

will apply the suggestion and re-merge it later.

crates/binstalk-git-repo-api/Cargo.toml Outdated Show resolved Hide resolved
crates/binstalk-fetchers/Cargo.toml Outdated Show resolved Hide resolved
Cargo.lock Outdated Show resolved Hide resolved
crates/binstalk/Cargo.toml Outdated Show resolved Hide resolved
@NobodyXu NobodyXu added this pull request to the merge queue Jun 10, 2024
Merged via the queue into main with commit 1dbd246 Jun 10, 2024
28 checks passed
@NobodyXu NobodyXu deleted the feat/private-github-repo branch June 10, 2024 06:15
@scullionw
Copy link

scullionw commented Jun 11, 2024

hi! sorry missed your comment:

i've tried and it doesn't seem to see the release? (it falls back to installing from source)

you can see this in the logs, which matches the screenshot below

DEBUG has_release_artifact{release=GhRelease { repo: GhRepo { owner: "TransitApp", repo: "gtfsRtProcessor" }, tag: "1.2.2" } artifact_name="gtfsrt-analyzer-v1.2.2-aarch64-apple-darwin.tar.gz"}: return=Ok(None)

releases:
Screenshot 2024-06-11 at 10 22 36 AM

full log: https://logpaste.com/UdzdiGd9

@NobodyXu
Copy link
Member Author

@scullionw what is the tag used?

If the tag is not v1.2.2 or 1.2.2, then automatic detection of cargo-binstall would not work, you would need to manually provide pkg-url.

@scullionw
Copy link

scullionw commented Jun 11, 2024

ah the tag is
Screenshot 2024-06-11 at 10 34 58 AM

basically crate-name + tag.

the issue is that this is a cargo workspace, so there could be more than one binary we want to release

edit: we can work around this for now though, and just release everything at the same time, we'll give it a try! thanks!

@NobodyXu
Copy link
Member Author

the issue is that this is a cargo workspace, so there could be more than one binary we want to release

@scullionw In that case, you can set the repository field in Cargo.toml to https://github.com/TransitApp/gtfsRtProcessor/tree/main/gtfsrt-analyzer

@scullionw
Copy link

And the other thing we notice with this PR is that it always asks for the github password (token) whether we provide the github token or not:
with GITHUB_TOKEN
Screenshot 2024-06-11 at 10 56 23 AM

without:
Screenshot 2024-06-11 at 10 55 45 AM

ideally, we would like the user to only have to provide the token once, and then it would be saved? What is your recommendation for this?

ive tried setting git config --global credential.helper osxkeychain to that it remembers but it changes nothing

@NobodyXu
Copy link
Member Author

Oh that's because you are using --git, cargo-binstall currently use gix to clone it, which would require username and password, if you didn't save it.

@NobodyXu
Copy link
Member Author

NobodyXu commented Jun 11, 2024

BTW we have a ticket to support private registry #1693

@scullionw
Copy link

Oh that's because you are using --git, cargo-binstall currently use gix to clone it, which would require username and password, if you didn't save it.

cool i will check how to save credentials with gitoxide

@NobodyXu
Copy link
Member Author

cool i will check how to save credentials with gitoxide

I think the default git-config one will work, it is compatible with git

@scullionw
Copy link

weird because gix works but not cargo-binstall 🤔
Screenshot 2024-06-11 at 11 17 47 AM

@NobodyXu
Copy link
Member Author

Ohhh I know, it could be that we configured gix to not use system-wide configuration

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.

BUG: cannot access private repositor with GITHUB_TOKEN provided
3 participants