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

Packaging github.com/aybabtme/humanlog [#1055740] #31

Closed
jmkim opened this issue Sep 6, 2023 · 29 comments
Closed

Packaging github.com/aybabtme/humanlog [#1055740] #31

jmkim opened this issue Sep 6, 2023 · 29 comments
Assignees
Labels
0.40.2 lazygit 0.40.2 packaging (2023~) ITP sent Intent To Package bug has been submitted ongoing Packaging started by someone RFS sent Request For Sponsorship bug has been submitted uploaded Package has been uploaded to Debian archive

Comments

@jmkim
Copy link
Collaborator

jmkim commented Sep 6, 2023

No description provided.

@jmkim jmkim added the 0.40.2 lazygit 0.40.2 packaging (2023~) label Sep 6, 2023
@anoopmsivadas
Copy link

Hi @jmkim , I have one doubt regarding the humanlog packaging. Lazygit uses Scanner API from an old version(v0.4.1) of humanlog. Humanlog no longer provides this function. So how can we handle this? we cannot package an old version for debian, right?

@jmkim
Copy link
Collaborator Author

jmkim commented Sep 19, 2023

Hi @anoopmsivadas,

Thanks for your careful work \o/

If in that case, we should ask @jesseduffield to update the lazygit for adopting the lastest version of humanlog.

@anoopmsivadas, to the lazygit repo, suppose:

  • you can file an issue about this
  • or fix yourself by sending a PR.

How do you think?

@anoopmsivadas
Copy link

@jmkim Then I'll try to fix this issue and raise a PR hopefully :) .

@jesseduffield
Copy link
Owner

Thanks @anoopmsivadas !

@jmkim jmkim added the ongoing Packaging started by someone label Sep 22, 2023
@anoopmsivadas
Copy link

The function signature of humanlog Scanner changed from func Scanner(src io.Reader, dst io.Writer, opts *HandlerOptions) error to func Scan(ctx context.Context, src io.Reader, sink sink.Sink, opts *HandlerOptions) error. Now it depends on two private packages accessible only to humanlog module. So I don't think a dependency upgrade would be possible.

@jmkim jmkim added the stuck Stuck (help needed) label Oct 4, 2023
@anoopmsivadas
Copy link

Hi @jmkim , @jesseduffield . Should we look for an alternative 👀 or integrate the old humanlog to lazygit 😬 !!

@anoopmsivadas
Copy link

Hi @jmkim, Antoine (Author of Humanlog) is fine with moving these internal dependency to public. But we will have to package/upgrade few more modules go.mod.

@WeepingClown13
Copy link

@anoopmsivadas
Copy link

@WeepingClown13 oh its cool. I can make a few more changes in upstream to ease the process. (have to find something else to package too 😅 )

@WeepingClown13
Copy link

WeepingClown13 commented Nov 10, 2023

>have to find something else to package too

I can help out with this as long as it is not complex ig. Which package?

Edit: I misread "something" as "someone" xD (offer still counts though!)

@jmkim
Copy link
Collaborator Author

jmkim commented Nov 10, 2023

Hey @anoopmsivadas, I saw today Maytham filed ITP for this package. I now figured out that you didn't send ITP yet.

I am not sure Maytham already discussed with you about filing the ITP.
(Because Maytham's purpose is for lazygit, I am guessing he already discussed with you privately?)

In Debian, ITP exposes the intention officially, so Maytham should take this package.

As I know, although you didn't send ITP yet, you are still digging the issue deeply on this package. I think you have to discuss with Maytham if you still have an interest for wrapping up this package.

@anoopmsivadas
Copy link

anoopmsivadas commented Nov 10, 2023

I already made some changes in upstream, was planning to send a PR for lazygit and then start from the humanlog dependencies. Since ITP is already sent by someone else they can package it. I'll make the changes in lazygit once the changes I made in humanlog releases. I might work on the humanlog dependencies as time permits. and will start with an ITP next time :)

Also please let me know if you need help with any other go packages :)

@Maytha8
Copy link

Maytha8 commented Nov 11, 2023

Hi there! I'm the guy that filed the ITP.

Since ITP is already sent by someone else they can package it.

If you'd like me to package it, I'm fine with that. I can also reassign the ITP to you, which is perfectly fine.

@Maytha8
Copy link

Maytha8 commented Nov 11, 2023

Regarding the humanlog dependencies, I've found that all of them have packages except for connectrpc.com/connect, which I can package if you'd like me to (I've already filed an ITP for this one as well.)

Note: github.com/bufbuild/connect-go is listed in the go.mod for humanlog, but it's been moved.

@jmkim
Copy link
Collaborator Author

jmkim commented Nov 11, 2023

I've found that all of them have packages except for connectrpc.com/connect

I added the issue for this: #41 \o/

@jmkim
Copy link
Collaborator Author

jmkim commented Nov 11, 2023

ITP : https://bugs.debian.org/1055740

@jmkim jmkim added the ITP sent Intent To Package bug has been submitted label Nov 11, 2023
@Maytha8
Copy link

Maytha8 commented Nov 11, 2023

@anoopmsivadas I'm happy for you to continue your packaging efforts if you'd like. If so, I will transfer the ITP over to you and transfer the packaging repo https://salsa.debian.org/go-team/packages/golang-github-humanlogio-humanlog over to you (it's a clean slate at the moment). I'm also happy to co-maintain the package with you if you'd like.
Let me know what you think. :)

@jmkim jmkim changed the title Packaging github.com/aybabtme/humanlog [#] Packaging github.com/aybabtme/humanlog [#1055740] Nov 15, 2023
@Maytha8
Copy link

Maytha8 commented Nov 15, 2023

I've got a finished package at this Salsa repo for golang-github-humanlogio-humanlog, but I haven't send an RFS yet since I'm awaiting it's dependencies to be uploaded to Debian (#41 and #42).

@WeepingClown13
Copy link

@Maytha8 maybe it is better to send the rfs and mention that you need the dependencies to be uploaded first, as it usually takes some time to get things running in the go team (I mean, I have 4 pending rfs there for a while now xD). Or maybe you can ask in the go team IRC channel whether that is a good idea.

@jmkim
Copy link
Collaborator Author

jmkim commented Nov 15, 2023

I've got a finished package at this Salsa repo

I cannot see the code o/

@Maytha8
Copy link

Maytha8 commented Nov 15, 2023

I cannot see the code o/

@jmkim sorry I meant https://salsa.debian.org/go-team/packages/golang-github-humanlogio-humanlog/ (forgot the github- part of the link).

@Maytha8
Copy link

Maytha8 commented Nov 15, 2023

@Maytha8 maybe it is better to send the rfs and mention that you need the dependencies to be uploaded first, as it usually takes some time to get things running in the go team (I mean, I have 4 pending rfs there for a while now xD). Or maybe you can ask in the go team IRC channel whether that is a good idea.

@WeepingClown13 I don't think there's any point in sending out an RFS for this package if the dependencies aren't there yet, since it can't be uploaded anyways without the deps being available. Let me know if you disagree.

I'm also in contact with Nilesh Patra (same person who reviewed your RFS) who's very active and will hopefully sponsor the packages soon if and when he has time.

@WeepingClown13
Copy link

WeepingClown13 commented Nov 15, 2023

@Maytha8 haha I was simply hoping that it could speed up the process, anything you think would work is fine :)
Hopefully Nilesh can take care of it. I hope he can take care of my rfs as well, I should've hinted I was about to do it when I met him two weeks ago xD He seems really busy these days.

@Maytha8
Copy link

Maytha8 commented Nov 17, 2023

Nilesh has uploaded #41 but not #42, for the following reason (below is an extract from my email reply):

On Fri, 2023-11-17 at 14:11 +0530, Nilesh Patra wrote:

This looks like really old code, and there's
golang-github-go-logfmt-logfmt in debian already.

Can the code in the target package be patched to use go-logfmt or is it
using specific features from kr-logfmt?

The target package (golang-github-humanlogio-humanlog[1][2]) uses the Handler
type from kr-logfmt for its own Handler type (handler.go), and this is the only
occurrence of kr-logfmt in the code.

golang-github-go-logfmt-logfmt doesn't have this Handler type that's being used,
and humanlog already uses go-logfmt as much as it can

I've made an issue upstream at humanlogio/humanlog#71 regarding this and I've
patched out the Handler type completely from humanlog for now
.

@Maytha8
Copy link

Maytha8 commented Nov 20, 2023

Both deps #41 and #42 have been uploaded

@Maytha8
Copy link

Maytha8 commented Nov 20, 2023

RFS sent

@Maytha8
Copy link

Maytha8 commented Nov 21, 2023

Delayed due to licensing issue, see humanlogio/api#1 for more info.

@anoopmsivadas anoopmsivadas removed their assignment Nov 23, 2023
@Maytha8
Copy link

Maytha8 commented Dec 8, 2023

Licensing issue fixed after an email to Antoine.
Now in NEW queue, awaiting ftpmasters' approval.

@jmkim jmkim added RFS sent Request For Sponsorship bug has been submitted uploaded Package has been uploaded to Debian archive and removed stuck Stuck (help needed) labels Dec 8, 2023
@Maytha8
Copy link

Maytha8 commented Dec 18, 2023

@jmkim Accepted! Can you please close this as done?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.40.2 lazygit 0.40.2 packaging (2023~) ITP sent Intent To Package bug has been submitted ongoing Packaging started by someone RFS sent Request For Sponsorship bug has been submitted uploaded Package has been uploaded to Debian archive
Projects
Status: Done
Development

No branches or pull requests

5 participants