warp-tech / warpgate

Smart SSH, HTTPS and MySQL bastion that requires no additional client-side software
Apache License 2.0
3.76k stars 115 forks source link

Merge request back log, help offer. #446

Open stappersg opened 1 year ago

stappersg commented 1 year ago

Hello Warp-tech people,

Currently there are 36 pending merge requests. Most of them are from @dependabot.

I'm offering help to merge those.

Benefit for the project: No more back log that screams this project is abonded

My benefit: Contributing to the success of this project.

Regards Geert Stappers My email addresses is stappers@stappers.it as documented in public visible part of my Github profile.

Eugeny commented 1 year ago

Hey, I normally just sort through these once a month - currently working on expanding the test suite before merging more. But it'd be awesome if you could go through these and merge relevant ones (based on release notes) into a single PR! (dependabot will automatically close individual PRs once it's merged)

stappersg commented 1 year ago

FWIW: I now know better what the task is for which I volunteered.

It looks like that failing tests are blocking the merge requests, it is a good thing.

I have to zoom in on errors like https://github.com/warp-tech/warpgate/actions/runs/3267742038/jobs/5373310834#step:9:197

Caused by:
  process didn't exit successfully: `/target/release/build/libc-dd803372746e72a1/build-script-build`
(exit status: 1)
  --- stderr
  /target/release/build/libc-dd803372746e72a1/build-script-build: /lib64/libc.so.6: version `GLIBC_2.18' not found
(required by /target/release/build/libc-dd803372746e72a1/build-script-build)
warning: build failed, waiting for other jobs to finish...
Error: The process '/home/runner/.cargo/bin/cross' failed with exit code 101

I will do that in other issue (or merge request).

Eugeny commented 1 year ago

This error is going to be caused by some library referencing v2.18+ version symbols in glibc. The builds are running against glibc 2.17 to retain compatibility with RHEL 7 - I would like to keep it that way unless there's some clear benefit.

Eugeny commented 1 year ago

Also something to keep in mind - there's a flaky test (test_ssh_proto.py::Test::test_direct_tcpip) that almost always fails in CI but doesn't locally - I haven't had time to debug it yet - but you can safely ignore it.

stappersg commented 1 year ago

almost always fails in CI ... but you can safely ignore it

I can and will ignore that failing test.

However, I think that dependabot doesn't do so, the reason for the merge request backlog ...

Eugeny commented 1 year ago

Dependabot doesn't merge anything automatically - it just creates PRs.

stappersg commented 1 year ago

Dependabot just creates PRs

I would like to have the privilege to approve merge request. Benefit for the warpgate project is (much) less backlog on MRs from @dependabot.

Eugeny commented 1 year ago

For starters, please just cherry-pick dependabot's commits into a single PR of your own - this way I'll have a chance to review it before merging.

stappersg commented 1 year ago

blunt github merge

For starters, please just cherry-pick dependabot's commits into a single PR of your own - this way I'll have a chance to review it before merging.

I think that the idea is avoid those ugly commit messages that github puts in. I like that idea and have started working on it.

For reviewing is this my advice

git remote add stappers https://git.sr.ht/~stappers/warpgate
git fetch stappers
git switch dependabotcherries
git log --oneline | head -n 15
git log --patch
ctrl-C on reaching Bump russh from 0.34.0-beta.16 to 0.34.0-beta.17

For merging the cherries is this my advice

git log --oneline | head -n 15  # for getting the short commit hashes
git switch main  # or an intermediate branch for extra check reasons
git cherry-pick 6fc6903 5c5d610 eebb6aa  # "oldest" three cherries first
git cherry-pick f7f80c4 52b3780 1caed9f 9ff967d 24be8d1 63e0856
git cherry-pick 77e2c09 ca7ded5 a69cdc0  # finish reverse order cherry picking

After your git push, please notify me (updating this issue is fine) and I will continue with this task for which I volunteered. Yeah, it are only 12 MRs of thirty something backlog, it is a "does it work as expected" run.

Eugeny commented 1 year ago
remote: fatal: failed to read object 90b544081ddc0ce40395800e25ec5ce900b3c761: Permission denied
remote: aborting due to possible repository corruption on the remote side.
fatal: protocol error: bad pack header

Also, are you checking the release notes as well? I'm not looking to bump every single library every time it has a new release - only when it's actually relevant. Picking two random MRs, neither once_cell nor futures have introduced any changes that are relevant for warpgate.

stappersg commented 1 year ago
remote: aborting due to possible repository corruption on the remote side.
fatal: protocol error: bad pack header

Probably a network hick-up.

Also, are you checking the release notes as well? I'm not looking to bump every single library every time it has a new release - only when it's actually relevant. Picking two random MRs, neither once_cell nor futures have introduced any changes that are relevant for warpgate.

No, I did not do that check on those first twelve.

(It is now midnigh in my timezone (CET, UTC+1), I'm going to sleep)

stappersg commented 1 year ago
remote: aborting due to possible repository corruption on the remote side.
fatal: protocol error: bad pack header

Probably a network hick-up.

Sadly not a hick-up. Reported at https://lists.sr.ht/~sircmpwn/sr.ht-discuss/%3C20221104072930.rqdcirclysp36ci5%40gpm.stappers.nl%3E

stappersg commented 1 year ago
remote: aborting due to possible repository corruption on the remote side.
fatal: protocol error: bad pack header

Probably a network hick-up.

Sadly not a hick-up. Reported at https://lists.sr.ht/~sircmpwn/sr.ht-discuss/%3C20221104072930.rqdcirclysp36ci5%40gpm.stappers.nl%3E

Now fixed \o/

stappers@uta:~/warpgate
$ git remote add stappers https://git.sr.ht/~stappers/warpgate
stappers@uta:~/warpgate
$ git fetch stappers
remote: Enumerating objects: 59, done.
remote: Counting objects: 100% (59/59), done.
remote: Compressing objects: 100% (48/48), done.
remote: Total 48 (delta 38), reused 0 (delta 0), pack-reused 0
Unpacking objects: 100% (48/48), 9.61 KiB | 265.00 KiB/s, done.
From https://git.sr.ht/~stappers/warpgate
 * [new branch]      dependabotcherries -> stappers/dependabotcherries
 * [new branch]      main               -> stappers/main
stappers@uta:~/warpgate
$ 
stappersg commented 1 year ago

The new dependabot merge requests had no effect on the ones that I have cherry picked. (Screenshot follows)

stappers@myhost:~/src/github/warpgate
$ git branch | grep ^\*
* dependabotcherries
stappers@myhost:~/src/github/warpgate
$ git log --oneline | head -n 15
0791136 Bump tracing-subscriber from 0.3.11 to 0.3.16
0fe3b97 Bump tracing from 0.1.35 to 0.1.37
c14eff2 Bump serde_yaml from 0.8.26 to 0.9.11
c065e93 Bump serde_json from 1.0.82 to 1.0.87
0e08411 Bump serde from 1.0.144 to 1.0.147
a144e55 Bump rustls from 0.20.6 to 0.20.7
ad01476 Bump openidconnect from 2.3.2 to 2.4.0
65dfc4b Bump once_cell from 1.14.0 to 1.16.0
b2e0958 Bump futures from 0.3.23 to 0.3.25
68a44ed Bump clap from 3.2.22 to 3.2.23
be0117b Bump async-trait from 0.1.56 to 0.1.58
a76da68 Bump anyhow from 1.0.62 to 1.0.66
bfa2d26 fixed #456 - postgresql migrations
891760f temporarily disable test_direct_tcpip
2b7baac lint, removed lazy_static
stappers@myhost:~/src/github/warpgate
$ git branch -r | grep upstream/dependabot/cargo
  upstream/dependabot/cargo/anyhow-1.0.66
  upstream/dependabot/cargo/async-trait-0.1.58
  upstream/dependabot/cargo/clap-3.2.23
  upstream/dependabot/cargo/futures-0.3.25
  upstream/dependabot/cargo/futures-core-0.3.25
  upstream/dependabot/cargo/futures-util-0.3.25
  upstream/dependabot/cargo/once_cell-1.16.0
  upstream/dependabot/cargo/openidconnect-2.4.0
  upstream/dependabot/cargo/poem-1.3.48
  upstream/dependabot/cargo/regex-1.7.0
  upstream/dependabot/cargo/russh-0.34.0
  upstream/dependabot/cargo/russh-keys-0.22.0
  upstream/dependabot/cargo/rustls-0.20.7
  upstream/dependabot/cargo/sea-orm-0.10.1
  upstream/dependabot/cargo/sea-orm-0.10.2
  upstream/dependabot/cargo/sea-orm-migration-0.10.1
  upstream/dependabot/cargo/sea-orm-migration-0.10.2
  upstream/dependabot/cargo/serde-1.0.147
  upstream/dependabot/cargo/serde_json-1.0.87
  upstream/dependabot/cargo/serde_yaml-0.9.11
  upstream/dependabot/cargo/time-0.3.16
  upstream/dependabot/cargo/time-0.3.17
  upstream/dependabot/cargo/totp-rs-3.1.0
  upstream/dependabot/cargo/tracing-0.1.37
  upstream/dependabot/cargo/tracing-core-0.1.30
  upstream/dependabot/cargo/tracing-subscriber-0.3.16
  upstream/dependabot/cargo/uuid-1.2.1
stappers@myhost:~/src/github/warpgate
$ 

It still makes sense to follow the advice from https://github.com/warp-tech/warpgate/issues/446#issuecomment-1302766017

stappersg commented 1 year ago

For starters, please just cherry-pick dependabot's commits into a single PR of your own - this way I'll have a chance to review it before merging.

I think that the idea is avoid those ugly commit messages that github puts in. I like that idea and have started working on it.

... remote git branch with picked cherries ...

Got followed up by:

... while checking that, got this...

remote: fatal: failed to read object 90b544081ddc0ce40395800e25ec5ce900b3c761: Permission denied
remote: aborting due to possible repository corruption on the remote side.
fatal: protocol error: bad pack header

Now fixed

plus some for both parties is this collaboration new.

What I would like to know:

Eugeny commented 1 year ago

@stappersg this is getting really awkward and I think you're having a wrong impression about how Dependabot works.

  1. You can avoid merge commits simply by using the merge & rebase button.
  2. There PRs are not ignored - I'm reviewing them as the time allows.
  3. Bumping every minor release of every dependency is not a goal of this project.
  4. Since you offered your help, I was hoping you could handle reading through the release notes and determining which PRs are relevant and which aren't. Cherry-picking them onto a single branch is just a cleanup/convenience thing that can be done in 60 seconds.
stappersg commented 1 year ago

Yes, Dependabot is new for me.

Acknowledge on Bumping every minor release of every dependency is not a goal of this project.

It is nice to see that now all the currently open merge requests do fit on a single page.

A "merge & rebase button" is is not visible to me.

Since you offered your help, I was hoping you could handle reading through the release notes and determining which PRs are relevant and which aren't.

Now working on #433

afbeelding

stappersg commented 1 year ago

This issue is not stalled, it documents Bumping every minor release of every dependency is not a goal of this project, it documents there is no backlog in pull requests.