[bug#47539,00/28] Add gh and dependencies

Message ID cover.1617262223.git.public@yoctocell.xyz
Headers show
Series Add gh and dependencies | expand

Message

Xinglu Chen April 1, 2021, 7:51 a.m. UTC
This patchset adds the 'gh' package, a CLI interface to GitHub.  The Go
importer did most of the work. :)

Some things to note:

- When building'gh', I couldn't get
  'go-github-com-charmbracelet-glamour' to build without moving some
  native-inputs to propagated-inputs, though, if I was only building
  'go-github-com-charmbracelet-glamour' it would build without moving
  things to propagated-inputs.  I am not sure why this happens.

- I found that (gnu packages syncthing) contained quite a few Go
  packages, I think it would make sense to move those packages to (gnu
  packages golang), and move the Syncthing package to (gnu packages
  sync).  This would be done in a separate commit though.

- The patch marked with 'D' adds a package which has been deprecated by
  upstream, but it is still needed by one of the packages that 'gh'
  depend on.  I have marked it as a hidden package, let me know if this
  is the right decision.

- The patch marked with 'W' adds a package which is needed by 'gh', but
  only used on Windows.  It is still Free Software (BSD-2), but I have
  also marked it as hidden.

Xinglu Chen (28):
  gnu: go-github-com-charmbracelet-glamour: Move some inputs to
    propagated-inputs.
  gnu: Add go-github-com-hinshun-vt10x.
  gnu: Add go-github-com-creack-pty.
D gnu: Add go-github-com-kr-pty.
  gnu: Add go-github-com-netflix-go-expect.
  gnu: Add go-github-com-alecaivazis-survey-v2.
  gnu: Add go-github-com-makenowjust-heredoc.
  gnu: Add go-github-com-briandowns-spinner.
  gnu: Add go-github-com-cli-browser.
  gnu: Add go-github-com-cli-oauth.
W gnu: Add go-github-com-cli-safeexec.
  gnu: Add go-github-com-russross-blackfriday-v2.
  gnu: Add go-github-com-shurcool-sanitized-anchor-name.
  gnu: Add go-github-com-cpuguy83-go-md2man-v2.
  gnu: Add go-github-com-enescakir-emoji.
  gnu: Add go-github-com-gabriel-vasile-mimetype.
  gnu: Add go-github-com-google-shlex.
  gnu: Add go-github-com-henvic-httpretty.
  gnu: Add go-github-com-itchyny-go-flags.
  gnu: Add go-github-com-itchyny-timefmt-go.
  gnu: Add go-github-com-itchyny-gojq.
  gnu: Add go-github-com-mattn-go-runewidth.
  gnu: Add go-github-com-rivo-uniseg.
  gnu: Add go-github-com-shurcool-githubv4.
  gnu: Add go-gopkg-in-yaml-v3.
  gnu: Add go-github-com-shurcool-graphql.
  gnu: Add go-github-com-cli-shurcool-graphql.
  gnu: Add gh.

 gnu/packages/golang.scm          | 745 ++++++++++++++++++++++++++++++-
 gnu/packages/version-control.scm |  92 ++++
 2 files changed, 826 insertions(+), 11 deletions(-)


base-commit: 94c77c9a0f7a1a4d7665d8fe566547016d2588d9

Comments

Leo Prikler April 1, 2021, 10:08 a.m. UTC | #1
Hi,

Am Donnerstag, den 01.04.2021, 09:51 +0200 schrieb Xinglu Chen:
> This patchset adds the 'gh' package, a CLI interface to GitHub.  The
> Go
> importer did most of the work. :)
I am not experienced with Go, so I can't comment on the rest too much,
but I wanted to point out, that single, double and triple letter
packages are (imo rightly) frowned upon in the Guix project. 
Considering, that this is the *official* Github CLI tool, a name like
"github-cli" would probably make more sense here.

> Some things to note:
> 
> - When building'gh', I couldn't get
>   'go-github-com-charmbracelet-glamour' to build without moving some
>   native-inputs to propagated-inputs, though, if I was only building
>   'go-github-com-charmbracelet-glamour' it would build without moving
>   things to propagated-inputs.  I am not sure why this happens.
Perhaps you need to add those native inputs as native inputs to gh as
well?  Try adding its native inputs to gh and see whether that changes
something.

> - I found that (gnu packages syncthing) contained quite a few Go
>   packages, I think it would make sense to move those packages to
> (gnu
>   packages golang), and move the Syncthing package to (gnu packages
>   sync).  This would be done in a separate commit though.
Indeed, that should probably done in a separate set.

> - The patch marked with 'D' adds a package which has been deprecated
> by
>   upstream, but it is still needed by one of the packages that 'gh'
>   depend on.  I have marked it as a hidden package, let me know if
> this
>   is the right decision.
Yes, we do so for other packages as well, that require outdated or
otherwise modified versions, that should not show up in user profiles
unless they really, really want to and use manifests to achieve their
goals.

> - The patch marked with 'W' adds a package which is needed by 'gh',
> but
>   only used on Windows.  It is still Free Software (BSD-2), but I
> have
>   also marked it as hidden.
This is at least in some sense an acceptable workaround, but how large
would the effort be to outright drop references to this package, given
that it's already not "actively" being used?

Regards,
Leo
Xinglu Chen April 1, 2021, 2:49 p.m. UTC | #2
On Thu, Apr 01 2021, Leo Prikler wrote:

> Hi,
>
> Am Donnerstag, den 01.04.2021, 09:51 +0200 schrieb Xinglu Chen:
>> This patchset adds the 'gh' package, a CLI interface to GitHub.  The
>> Go importer did most of the work. :)
> I am not experienced with Go, so I can't comment on the rest too much,
> but I wanted to point out, that single, double and triple letter
> packages are (imo rightly) frowned upon in the Guix project. 
> Considering, that this is the *official* Github CLI tool, a name like
> "github-cli" would probably make more sense here.

Oh yeah, I saw the thread on guix-devel, I should have thought of this.

>> - When building'gh', I couldn't get
>>   'go-github-com-charmbracelet-glamour' to build without moving some
>>   native-inputs to propagated-inputs, though, if I was only building
>>   'go-github-com-charmbracelet-glamour' it would build without moving
>>   things to propagated-inputs.  I am not sure why this happens.
> Perhaps you need to add those native inputs as native inputs to gh as
> well?  Try adding its native inputs to gh and see whether that changes
> something.

I will try and report back.

>> - The patch marked with 'D' adds a package which has been deprecated
>>   by upstream, but it is still needed by one of the packages that 'gh'
>>   depend on.  I have marked it as a hidden package, let me know if this
>>   is the right decision.
> Yes, we do so for other packages as well, that require outdated or
> otherwise modified versions, that should not show up in user profiles
> unless they really, really want to and use manifests to achieve their
> goals.

Good to know, I will have to mark 'go-github-com-cli-shurcool-graphql'
as hidden as well since it's GitHubs' fork of
'go-github-com-shurcool-graphql' and most likely only used by 'gh'.

>> - The patch marked with 'W' adds a package which is needed by 'gh',
>>   but only used on Windows.  It is still Free Software (BSD-2), but I
>>   have also marked it as hidden.
> This is at least in some sense an acceptable workaround, but how large
> would the effort be to outright drop references to this package, given
> that it's already not "actively" being used?

I am not familiar with the Go ecosystem so I don't know if this will be
possible, but I will look into it.

Thank you for the rewiew!
Leo Prikler April 1, 2021, 4:20 p.m. UTC | #3
Hi,
Am Donnerstag, den 01.04.2021, 16:49 +0200 schrieb Xinglu Chen:
> > > - The patch marked with 'D' adds a package which has been
> > > deprecated
> > >   by upstream, but it is still needed by one of the packages that
> > > 'gh'
> > >   depend on.  I have marked it as a hidden package, let me know
> > > if this
> > >   is the right decision.
> > Yes, we do so for other packages as well, that require outdated or
> > otherwise modified versions, that should not show up in user
> > profiles
> > unless they really, really want to and use manifests to achieve
> > their
> > goals.
> 
> Good to know, I will have to mark 'go-github-com-cli-shurcool-
> graphql'
> as hidden as well since it's GitHubs' fork of
> 'go-github-com-shurcool-graphql' and most likely only used by 'gh'.
Just to clarify, I don't think that hiding will be necessary for this
package (since it differs by name), but it might still be a good idea
to hide it if you have a justification for doing so.  That said, your
judgement on this matter is probably going to be better than mine.

Regards,
Leo
Xinglu Chen April 1, 2021, 4:25 p.m. UTC | #4
On Thu, Apr 01 2021, Xinglu Chen wrote:

>>> - When building'gh', I couldn't get
>>>   'go-github-com-charmbracelet-glamour' to build without moving some
>>>   native-inputs to propagated-inputs, though, if I was only building
>>>   'go-github-com-charmbracelet-glamour' it would build without moving
>>>   things to propagated-inputs.  I am not sure why this happens.
>> Perhaps you need to add those native inputs as native inputs to gh as
>> well?  Try adding its native inputs to gh and see whether that changes
>> something.
>
> I will try and report back.

Adding the packages as native-inputs for github-cli fixed the problem.
Thank you!

>>> - The patch marked with 'W' adds a package which is needed by 'gh',
>>>   but only used on Windows.  It is still Free Software (BSD-2), but I
>>>   have also marked it as hidden.
>> This is at least in some sense an acceptable workaround, but how large
>> would the effort be to outright drop references to this package, given
>> that it's already not "actively" being used?
>
> I am not familiar with the Go ecosystem so I don't know if this will be
> possible, but I will look into it.

Compiling github-cli without the package doesn't work, and the package
is used in multiple places in the source code.  I don't really know what
we can do here.  Should I open an issue on their GitHub page, or just
leave it as is?
Leo Prikler April 1, 2021, 4:47 p.m. UTC | #5
Am Donnerstag, den 01.04.2021, 18:25 +0200 schrieb Xinglu Chen:
> On Thu, Apr 01 2021, Xinglu Chen wrote:
> 
> > > > - When building'gh', I couldn't get
> > > >   'go-github-com-charmbracelet-glamour' to build without moving
> > > > some
> > > >   native-inputs to propagated-inputs, though, if I was only
> > > > building
> > > >   'go-github-com-charmbracelet-glamour' it would build without
> > > > moving
> > > >   things to propagated-inputs.  I am not sure why this happens.
> > > Perhaps you need to add those native inputs as native inputs to
> > > gh as
> > > well?  Try adding its native inputs to gh and see whether that
> > > changes
> > > something.
> > 
> > I will try and report back.
> 
> Adding the packages as native-inputs for github-cli fixed the
> problem.
> Thank you!
You're welcome.

> > > > - The patch marked with 'W' adds a package which is needed by
> > > > 'gh',
> > > >   but only used on Windows.  It is still Free Software (BSD-2), 
> > > > but I
> > > >   have also marked it as hidden.
> > > This is at least in some sense an acceptable workaround, but how
> > > large
> > > would the effort be to outright drop references to this package,
> > > given
> > > that it's already not "actively" being used?
> > 
> > I am not familiar with the Go ecosystem so I don't know if this
> > will be
> > possible, but I will look into it.
> 
> Compiling github-cli without the package doesn't work, and the
> package
> is used in multiple places in the source code.  I don't really know
> what
> we can do here.  Should I open an issue on their GitHub page, or just
> leave it as is?
For now, I don't think this is going to be a blocking issue, as long as
the package and the input are appropriately marked, e.g. with comments,
and we make sure not to endorse the Microsoft OS through them.  
As to how we might handle this, we could do our own investigations into
dropping the dependency, but asking a question (make sure to tag it as
such) should work as well.  Also be clear, that the goal is not
necessarily to remove the dependency altogether, but rather make it an
optional one.

After doing some quick investigations myself, I think the following
pair of substitute*s might work:
  (("github.com/cli/safeexec") "os/exec")
  (("safeexec") "exec")

Regards,
Leo
Jack Hill April 1, 2021, 5:21 p.m. UTC | #6
On Thu, 1 Apr 2021, Leo Prikler wrote:

> After doing some quick investigations myself, I think the following
> pair of substitute*s might work:
>  (("github.com/cli/safeexec") "os/exec")
>  (("safeexec") "exec")

I happened to be reading the Go blog post about command path security [0]. 
I haven't looked at it very closely, but I'm hopeful that future gh 
versions may be able to get safeexec-like behavior from the Go standard 
library.

[0] https://blog.golang.org/path-security

Best,
Jack
Xinglu Chen April 1, 2021, 6:05 p.m. UTC | #7
On Thu, Apr 01 2021, Leo Prikler wrote:

>> Compiling github-cli without the package doesn't work, and the
>> package is used in multiple places in the source code.  I don't
>> really know what we can do here.  Should I open an issue on their
>> GitHub page, or just leave it as is?
> For now, I don't think this is going to be a blocking issue, as long as
> the package and the input are appropriately marked, e.g. with comments,
> and we make sure not to endorse the Microsoft OS through them.  
> As to how we might handle this, we could do our own investigations into
> dropping the dependency, but asking a question (make sure to tag it as
> such) should work as well.  Also be clear, that the goal is not
> necessarily to remove the dependency altogether, but rather make it an
> optional one.
>
> After doing some quick investigations myself, I think the following
> pair of substitute*s might work:
>   (("github.com/cli/safeexec") "os/exec")
>   (("safeexec") "exec")

Thank you for the pointer, I managed to get it to work with the
following snippet:

#+begin_src scheme
(add-after 'unpack 'remove-safeexec
           (lambda* (#:key outputs #:allow-other-keys)
             (let ((prefix-file (lambda (file)
                             (string-append "src/github.com/cli/cli/" file))))
               ;; 'github.com/cli/safeexec' is only used for Windows, we
               ;; replace it with the regular 'os/exec'.  See
               ;; <https://issues.guix.gnu.org/47539> for discussion.
               (substitute* (map prefix-file
                                 '("pkg/cmd/alias/expand/expand.go"
                                   "script/build.go"))
                 (("github.com/cli/safeexec") "os/exec")
                 (("safeexec") "exec"))
               ;; These files have already imported 'os/exec', meaning that
               ;; 'os/exec' would get imported twice, causing an error.
               ;; Instead, we just remove the 'github.com/cli/safeexec'.
               (substitute* (map prefix-file
                                 '("cmd/gh/main.go"
                                   "git/git.go"
                                   "pkg/iostreams/iostreams.go"
                                   "pkg/cmd/auth/shared/ssh_keys.go"
                                   "pkg/cmd/pr/checkout/checkout.go"
                                   "pkg/cmdutil/web_browser.go"
                                   "pkg/surveyext/editor_manual.go"))
                 (("\"github.com/cli/safeexec\"") "")
                 (("safeexec") "exec")))))
#+end_src

I also used it to open an issue and everything worked as expected.
Leo Prikler April 1, 2021, 6:10 p.m. UTC | #8
Am Donnerstag, den 01.04.2021, 20:05 +0200 schrieb Xinglu Chen:
> On Thu, Apr 01 2021, Leo Prikler wrote:
> 
> > > Compiling github-cli without the package doesn't work, and the
> > > package is used in multiple places in the source code.  I don't
> > > really know what we can do here.  Should I open an issue on their
> > > GitHub page, or just leave it as is?
> > For now, I don't think this is going to be a blocking issue, as
> > long as
> > the package and the input are appropriately marked, e.g. with
> > comments,
> > and we make sure not to endorse the Microsoft OS through them.  
> > As to how we might handle this, we could do our own investigations
> > into
> > dropping the dependency, but asking a question (make sure to tag it
> > as
> > such) should work as well.  Also be clear, that the goal is not
> > necessarily to remove the dependency altogether, but rather make it
> > an
> > optional one.
> > 
> > After doing some quick investigations myself, I think the following
> > pair of substitute*s might work:
> >   (("github.com/cli/safeexec") "os/exec")
> >   (("safeexec") "exec")
> 
> Thank you for the pointer, I managed to get it to work with the
> following snippet:
> 
> #+begin_src scheme
> (add-after 'unpack 'remove-safeexec
>            (lambda* (#:key outputs #:allow-other-keys)
>              (let ((prefix-file (lambda (file)
>                              (string-append "src/github.com/cli/cli/"
> file))))
>                ;; 'github.com/cli/safeexec' is only used for Windows,
> we
>                ;; replace it with the regular 'os/exec'.  See
>                ;; <https://issues.guix.gnu.org/47539> for discussion.
>                (substitute* (map prefix-file
>                                  '("pkg/cmd/alias/expand/expand.go"
>                                    "script/build.go"))
>                  (("github.com/cli/safeexec") "os/exec")
>                  (("safeexec") "exec"))
>                ;; These files have already imported 'os/exec',
> meaning that
>                ;; 'os/exec' would get imported twice, causing an
> error.
>                ;; Instead, we just remove the
> 'github.com/cli/safeexec'.
>                (substitute* (map prefix-file
>                                  '("cmd/gh/main.go"
>                                    "git/git.go"
>                                    "pkg/iostreams/iostreams.go"
>                                    "pkg/cmd/auth/shared/ssh_keys.go"
>                                    "pkg/cmd/pr/checkout/checkout.go"
>                                    "pkg/cmdutil/web_browser.go"
>                                    "pkg/surveyext/editor_manual.go"))
>                  (("\"github.com/cli/safeexec\"") "")
>                  (("safeexec") "exec")))))
> #+end_src
This should probably be implemented with directory excursions, but
other than that if it works for you, then LGTM :)
Xinglu Chen April 1, 2021, 6:16 p.m. UTC | #9
On Thu, Apr 01 2021, Leo Prikler wrote:

> Am Donnerstag, den 01.04.2021, 20:05 +0200 schrieb Xinglu Chen:
>> 
>> Thank you for the pointer, I managed to get it to work with the
>> following snippet:
>> 
>> #+begin_src scheme
>> (add-after 'unpack 'remove-safeexec
>>            (lambda* (#:key outputs #:allow-other-keys)
>>              (let ((prefix-file (lambda (file)
>>                              (string-append "src/github.com/cli/cli/"
>> file))))
>>                ;; 'github.com/cli/safeexec' is only used for Windows,
>> we
>>                ;; replace it with the regular 'os/exec'.  See
>>                ;; <https://issues.guix.gnu.org/47539> for discussion.
>>                (substitute* (map prefix-file
>>                                  '("pkg/cmd/alias/expand/expand.go"
>>                                    "script/build.go"))
>>                  (("github.com/cli/safeexec") "os/exec")
>>                  (("safeexec") "exec"))
>>                ;; These files have already imported 'os/exec',
>> meaning that
>>                ;; 'os/exec' would get imported twice, causing an
>> error.
>>                ;; Instead, we just remove the
>> 'github.com/cli/safeexec'.
>>                (substitute* (map prefix-file
>>                                  '("cmd/gh/main.go"
>>                                    "git/git.go"
>>                                    "pkg/iostreams/iostreams.go"
>>                                    "pkg/cmd/auth/shared/ssh_keys.go"
>>                                    "pkg/cmd/pr/checkout/checkout.go"
>>                                    "pkg/cmdutil/web_browser.go"
>>                                    "pkg/surveyext/editor_manual.go"))
>>                  (("\"github.com/cli/safeexec\"") "")
>>                  (("safeexec") "exec")))))
>> #+end_src
> This should probably be implemented with directory excursions, but
> other than that if it works for you, then LGTM :)

Ah, I didn't know about that.  Thanks for pointing this out. :)
Xinglu Chen April 1, 2021, 6:31 p.m. UTC | #10
Changes since v1:

- Renamed 'gh' to 'github-cli' to avoid short two or three letter names.

- Added a phase in 'github-cli' to remove its dependence on the
  'go-github-com-cli-safeexec' package, and I also removed
  'go-github-com-cli-safeexec' itself since it is only used on Windows.

- Removed 'go-github-com-shurcool-sanitized-anchor-name' because it
  already existed.  Upstream uses '_' instead of '-' in the name, the Go
  importer didn't convert '_' to '-' so I had to fix it myself without
  realizing that the package already existed.
  
- Marked 'go-github-com-cli-shurcool-graphql' as a hidden package
  because it is GitHub's own fork of 'go-github-com-shurcool-graphql',
  and is only used by 'github-cli'.
  
Xinglu Chen (26):
  gnu: Add go-github-com-hinshun-vt10x.
  gnu: Add go-github-com-creack-pty.
  gnu: Add go-github-com-kr-pty.
  gnu: Add go-github-com-netflix-go-expect.
  gnu: Add go-github-com-alecaivazis-survey-v2.
  gnu: Add go-github-com-makenowjust-heredoc.
  gnu: Add go-github-com-briandowns-spinner.
  gnu: Add go-github-com-cli-browser.
  gnu: Add go-github-com-cli-oauth.
  gnu: Add go-github-com-cli-safeexec.
  gnu: Add go-github-com-russross-blackfriday-v2.
  gnu: Add go-github-com-cpuguy83-go-md2man-v2.
  gnu: Add go-github-com-enescakir-emoji.
  gnu: Add go-github-com-gabriel-vasile-mimetype.
  gnu: Add go-github-com-google-shlex.
  gnu: Add go-github-com-henvic-httpretty.
  gnu: Add go-github-com-itchyny-go-flags.
  gnu: Add go-github-com-itchyny-timefmt-go.
  gnu: Add go-github-com-itchyny-gojq.
  gnu: Add go-github-com-mattn-go-runewidth.
  gnu: Add go-github-com-rivo-uniseg.
  gnu: Add go-github-com-shurcool-githubv4.
  gnu: Add go-gopkg-in-yaml-v3.
  gnu: Add go-github-com-shurcool-graphql.
  gnu: Add go-github-com-cli-shurcool-graphql.
  gnu: Add github-cli.

 gnu/packages/golang.scm          | 697 +++++++++++++++++++++++++++++++
 gnu/packages/version-control.scm | 145 +++++++
 2 files changed, 842 insertions(+)


base-commit: 9c9a36de732ea8485e47e84b888e121233f68452
Sharlatan Hellseher April 19, 2021, 10:37 p.m. UTC | #11
For the Glang package where version is not defined

--8<---------------cut here---------------start------------->8---
+(define-public go-github-com-shurcool-githubv4
+ (package+ (name "go-github-com-shurcool-githubv4")
+ (version "0.0.0-20201206200315-234843c633fa")
--8<---------------cut here---------------start------------->8---

It's better to use latest commit (check any of gnu/packages/golang.scm)

--8<---------------cut here---------------start------------->8---
(define-public go-golang-org-x-image
  (let ((commit "58c23975cae11f062d4b3b0c143fe248faac195d")
        (revision "1"))
    (package
      (name "go-golang-org-x-image")
      (version (git-version "0.0.0" revision commit))
--8<---------------cut here---------------start------------->8---

-- 
… наш разум - превосходная объяснительная машина которая способна
найти смысл почти в чем угодно, истолковать любой феномен, но
совершенно не в состоянии принять мысль о непредсказуемости.
Xinglu Chen April 20, 2021, 5:26 a.m. UTC | #12
On Mon, Apr 19 2021, Sharlatan Hellseher wrote:

> For the Glang package where version is not defined
>
> --8<---------------cut here---------------start------------->8---
> +(define-public go-github-com-shurcool-githubv4
> + (package+ (name "go-github-com-shurcool-githubv4")
> + (version "0.0.0-20201206200315-234843c633fa")
> --8<---------------cut here---------------start------------->8---
>
> It's better to use latest commit (check any of gnu/packages/golang.scm)

The github-cli package specifies an exact revision for some of its
dependencies[1], and thats what the Go importer generated.  Should I use
the latest commit anyway?

[1]: https://github.com/cli/cli/blob/trunk/go.mod
Leo Famulari April 20, 2021, 5:34 a.m. UTC | #13
On Tue, Apr 20, 2021 at 07:26:59AM +0200, Xinglu Chen wrote:
> The github-cli package specifies an exact revision for some of its
> dependencies[1], and thats what the Go importer generated.  Should I use
> the latest commit anyway?
> 
> [1]: https://github.com/cli/cli/blob/trunk/go.mod

You should always use what's specified in go.mod.

Golang software is like that: You should use exact versions of
dependencies.
Xinglu Chen April 20, 2021, 7:34 a.m. UTC | #14
On Tue, Apr 20 2021, Leo Famulari wrote:

> On Tue, Apr 20, 2021 at 07:26:59AM +0200, Xinglu Chen wrote:
>> The github-cli package specifies an exact revision for some of its
>> dependencies[1], and thats what the Go importer generated.  Should I use
>> the latest commit anyway?
>> 
>> [1]: https://github.com/cli/cli/blob/trunk/go.mod
>
> You should always use what's specified in go.mod.
>
> Golang software is like that: You should use exact versions of
> dependencies.

Ok, that’s what I did. :)