diff mbox series

[bug#57891,1/1] gnu: Add emacs-org-roam-ui.

Message ID 20220917224158.916761-1-plattfot@posteo.net
State Accepted
Headers show
Series Add emacs-org-roam-ui | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git-branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Fredrik Salomonsson Sept. 17, 2022, 10:41 p.m. UTC
* gnu/packages/emacs-xyz.scm (emacs-org-roam-ui): New variable.
---
 gnu/packages/emacs-xyz.scm | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

Comments

Nicolas Goaziou Sept. 18, 2022, 9:29 a.m. UTC | #1
Hello,

Fredrik Salomonsson <plattfot@posteo.net> writes:

> * gnu/packages/emacs-xyz.scm (emacs-org-roam-ui): New variable.

Thank you. Applied with the following changes.

> +(define-public emacs-org-roam-ui
> +  (package
> +    (name "emacs-org-roam-ui")
> +    (version "20220803.1024")

Version is actually "0.1".
> +    (source
> +      (origin
> +        (method git-fetch)
> +        (uri (git-reference
> +               (url "https://github.com/org-roam/org-roam-ui.git")

I removed ".git" suffix.
> +               (commit "c75fc7506ee7f03840a9a93ed9336d7ed24551aa")))
> +        (sha256
> +          (base32 "0mkcd2622np8s5qz2zvx7lch6dc586xqmn6914gi4ym7nvklf3zy"))))
> +    (build-system emacs-build-system)
> +    (propagated-inputs
> +     (list
> +      emacs-org-roam
> +      emacs-simple-httpd
> +      emacs-websocket))
> +    (arguments
> +      (list
> +       #:include #~(cons "^out" %default-include)))
> +    (home-page "https://github.com/org-roam/org-roam-ui")
> +    (synopsis "Web User Interface for Org-roam")

I capitalzed project name Org Roam.

> +    (description
> +      "Org-Roam-UI is meant as a successor of org-roam-server that extends
> +functionality of Org Roam with a Web app that runs side-by-side with Emacs.
> +Providing a web interface for navigating around notes created within
> Org Roam.")

I also capitalized project names. I changed "Web app" into "web app",
but I think it should be "web application". I eventually merged the two
into a single one.

Regards,
Nicolas Graves Sept. 18, 2022, 4:16 p.m. UTC | #2
IIRC, there's an issue with packaging guidelines because the package
contains minified js.

I'm not sure that's possible, but we could try to re-generate those
minified js files in a sub-package or something like that.

The number of dependencies is however huge and contains react in
package.json, it probably comes down to a problem as difficult as the
bootstrapping of any big node package.

Maybe we could contact the author and see if there's a way to decrease
the number of dependencies and the dependency on react so that we can
work to make the package follow the packaging guidelines?

--
Best regards,
Nicolas Graves
Fredrik Salomonsson Sept. 18, 2022, 5:43 p.m. UTC | #3
Thank you for merging and fixing all the minor mistakes I made.

Nicolas Graves <ngraves@ngraves.fr> writes:

> IIRC, there's an issue with packaging guidelines because the package
> contains minified js.
>
> I'm not sure that's possible, but we could try to re-generate those
> minified js files in a sub-package or something like that.
>

I couldn't find anything about this in the guidelines. But creating
separate package(s) for this sounds like more of a guix way of doing it.

> The number of dependencies is however huge and contains react in
> package.json, it probably comes down to a problem as difficult as the
> bootstrapping of any big node package.

I have very little experiense with javascript, so I completely missed
the massive dependency list it depends on, sorry about that.

> Maybe we could contact the author and see if there's a way to decrease
> the number of dependencies and the dependency on react so that we can
> work to make the package follow the packaging guidelines?

I'll open up an issue and ask. But should this package be pulled from
guix, given that it might be breaking some guidelines?
M Sept. 18, 2022, 7:09 p.m. UTC | #4
On 18-09-2022 19:43, Fredrik Salomonsson wrote:
> Nicolas Graves<ngraves@ngraves.fr>  writes:
> 
>> IIRC, there's an issue with packaging guidelines because the package
>> contains minified js.
>>
>> I'm not sure that's possible, but we could try to re-generate those
>> minified js files in a sub-package or something like that.
>>
> I couldn't find anything about this in the guidelines.

Some relevant parts (mostly, it boils down to 'minified js' not counting 
as source code, but binaries):

  * (guix)Introduction:

    [...] Guix makes it easy [...] to build packages from source, [...]

  * (guix)Binary Installation

    Note: If you do not enable substitutes, Guix will end up
    building _everything_ from source on your machine,

  * (guix)Features

    When a pre-built binary for a ‘/gnu/store’ item is
    available from an external source—a “substitute”, Guix just downloads
    it and unpacks it; otherwise, it builds the package from source,

  * (guix)Additional Build Options

    Fetch and return the source of PACKAGE-OR-DERIVATION [...]

  * (guix) Snippets versus Phases
    Origin snippets are typically used to remove unwanted files such as
    bundled libraries, [...]


  * (guix)Submitting Patches

   8. Make sure the package does not use bundled copies of software
      already available as separate packages.

      Sometimes, packages include copies of the source code of their
      dependencies as a convenience for users.  However, as a
      distribution, we want to make sure that such packages end up using
      the copy we already have in the distribution, if there is one.
      This [...], and allows the distribution to make transverse changes
      such as applying security updates for a given software package in a
      single place and have them affect the whole system—something that
      bundled copies prevent.

   ‘Minified JS that we don't know how to rebuild’ doesn't mesh well with
   such 'transverse changes' and "guix build --source will return the
   source code".

Greetings,
Maxime.
Fredrik Salomonsson Sept. 18, 2022, 11:24 p.m. UTC | #5
Maxime Devos <maximedevos@telenet.be> writes:

> On 18-09-2022 19:43, Fredrik Salomonsson wrote:
>> Nicolas Graves<ngraves@ngraves.fr>  writes:
>> 
>>> IIRC, there's an issue with packaging guidelines because the package
>>> contains minified js.
>>>
>>> I'm not sure that's possible, but we could try to re-generate those
>>> minified js files in a sub-package or something like that.
>>>
>> I couldn't find anything about this in the guidelines.
>
> Some relevant parts (mostly, it boils down to 'minified js' not counting 
> as source code, but binaries):
>
>   * (guix)Introduction:
>
>     [...] Guix makes it easy [...] to build packages from source, [...]
>
>   * (guix)Binary Installation
>
>     Note: If you do not enable substitutes, Guix will end up
>     building _everything_ from source on your machine,
>
>   * (guix)Features
>
>     When a pre-built binary for a ‘/gnu/store’ item is
>     available from an external source—a “substitute”, Guix just downloads
>     it and unpacks it; otherwise, it builds the package from source,
>
>   * (guix)Additional Build Options
>
>     Fetch and return the source of PACKAGE-OR-DERIVATION [...]
>
>   * (guix) Snippets versus Phases
>     Origin snippets are typically used to remove unwanted files such as
>     bundled libraries, [...]
>
>
>   * (guix)Submitting Patches
>
>    8. Make sure the package does not use bundled copies of software
>       already available as separate packages.
>
>       Sometimes, packages include copies of the source code of their
>       dependencies as a convenience for users.  However, as a
>       distribution, we want to make sure that such packages end up using
>       the copy we already have in the distribution, if there is one.
>       This [...], and allows the distribution to make transverse changes
>       such as applying security updates for a given software package in a
>       single place and have them affect the whole system—something that
>       bundled copies prevent.
>
>    ‘Minified JS that we don't know how to rebuild’ doesn't mesh well with
>    such 'transverse changes' and "guix build --source will return the
>    source code".

Ah, I didn't realize minified JS was generated and should be classified
as binaries. Or even that minified JS existed before it was pointed out
to me in this thread. It makes sense now though, thank you for clearing
that up.

Then I think it is probably best that this package gets removed from
guix, until it can be packaged properly.
Nicolas Goaziou Sept. 19, 2022, 10:24 a.m. UTC | #6
Hello,

Fredrik Salomonsson <plattfot@posteo.net> writes:

> Ah, I didn't realize minified JS was generated and should be classified
> as binaries. Or even that minified JS existed before it was pointed out
> to me in this thread. It makes sense now though, thank you for clearing
> that up.
>
> Then I think it is probably best that this package gets removed from
> guix, until it can be packaged properly.

I reverted the commit.

Regards,
Fredrik Salomonsson Sept. 19, 2022, 6:07 p.m. UTC | #7
Hi,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Hello,
>
> Fredrik Salomonsson <plattfot@posteo.net> writes:
>
>> Ah, I didn't realize minified JS was generated and should be classified
>> as binaries. Or even that minified JS existed before it was pointed out
>> to me in this thread. It makes sense now though, thank you for clearing
>> that up.
>>
>> Then I think it is probably best that this package gets removed from
>> guix, until it can be packaged properly.
>
> I reverted the commit.

Thanks.

I asked the author about reducing the dependencies, but they did not
think it was likely given that most of this package is written in
typescript.
diff mbox series

Patch

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index d99df6cc69..e41e820761 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -30717,6 +30717,35 @@  (define-public emacs-org-roam
 personal wiki.")
     (license license:gpl3+)))
 
+(define-public emacs-org-roam-ui
+  (package
+    (name "emacs-org-roam-ui")
+    (version "20220803.1024")
+    (source
+      (origin
+        (method git-fetch)
+        (uri (git-reference
+               (url "https://github.com/org-roam/org-roam-ui.git")
+               (commit "c75fc7506ee7f03840a9a93ed9336d7ed24551aa")))
+        (sha256
+          (base32 "0mkcd2622np8s5qz2zvx7lch6dc586xqmn6914gi4ym7nvklf3zy"))))
+    (build-system emacs-build-system)
+    (propagated-inputs
+     (list
+      emacs-org-roam
+      emacs-simple-httpd
+      emacs-websocket))
+    (arguments
+      (list
+       #:include #~(cons "^out" %default-include)))
+    (home-page "https://github.com/org-roam/org-roam-ui")
+    (synopsis "Web User Interface for Org-roam")
+    (description
+      "Org-Roam-UI is meant as a successor of org-roam-server that extends
+functionality of Org Roam with a Web app that runs side-by-side with Emacs.
+Providing a web interface for navigating around notes created within Org Roam.")
+    (license license:gpl3+)))
+
 (define-public emacs-org-roam-bibtex
   (package
     (name "emacs-org-roam-bibtex")