guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: gnu: Add syncthing.


From: Petter
Subject: Re: gnu: Add syncthing.
Date: Fri, 16 Dec 2016 13:48:57 +0100

On 2016-12-16 02:52, Leo Famulari wrote:
On Thu, Dec 15, 2016 at 10:06:59PM +0100, Petter wrote:
Hi again,

I've prefixed most of the packages with "golang-" now. However, there are
some
packages already starting with "golang-", f.ex.
"golang-org-x-text-unicode-norm",
and I left those alone. It's inconsistent, but I felt this was preferable to
names like "golang-golang-org-x-text-unicode-norm". What do you think?

That's the right way. It fits the package naming guidelines:

https://www.gnu.org/software/guix/manual/html_node/Package-Naming.html

I'm able to interpret your answer both ways, should I make it
"golang-golang-org-x-text-unicode-norm"?

Finally, there's a telemetry configuration in Syncthing. It is opt-in;
but it will ask the user after a few hours if they want to join. The
plan is to disable the question, however I suspect I've messed up the
build system in that area, so this takes some more looking in to.

Personally, I'm fine with the upstream "opt-in" nag warning. It only
shows up when you open the Syncthing web interface; it's not an
intrusive desktop "notification". Once the user has said "yes" or "no",
it doesn't appear again until Syncthing makes a change to what
information they collect.

But, if people think our package should never ask, I don't mind if we
disable the request, as long as we share our changes with the Syncthing
project and they don't notice anything broken.

I'll make an attempt at this while the jury is out, primarily to
retrieve the ability to edit files. (Either I broke it, or I need to
learn how to use (snippet).

Many of my comments below are about tedious things. Let us know if you
are getting sick of working on these packages, and I will help :) This
includes improving the descriptions.

Yes! Help please. The meta-data part is tricky and time consuming, I'd
rather spend my Guix time on the build system than descriptions
etc. Synopsis and descriptions are just stuff i found on their
home-page.

Date: Thu, 15 Dec 2016 21:42:03 +0100
Subject: [PATCH] gnu: Add Syncthing.

* gnu/packages/syncthing.scm: New file.

Cool!

+(define-public syncthing

+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/syncthing/syncthing/";)
+                    (commit (string-append "v" version))))
+ (file-name (string-append name "-" version "-checkout"))

We should use the Syncthing release tarball:

https://github.com/syncthing/syncthing/releases/download/v0.14.14/syncthing-source-v0.14.14.tar.gz

Totally agree! However, tarballs are currently unsupported... due to,
hm, let's say "because very good reasons noone can be blamed for,
particularly and especially not me".

Ok, I messed up the build system :P Removed too much code
apparently. I'm working on getting this back.

+    (arguments
+     `(#:import-path "github.com/syncthing/syncthing"

What do you think about having the go-build-system try to automatically
generate the import-path based on the source URL, with the option for
the packager to set it manually, as shown here?

For many of the packages in this patch (which will eventually be split
into one package per patch ;) ), that auto-generated import-path
could be correct.

I think that an (arguments) field indicates that the package's build
scripts have deviated from the standard. If a Guix build system requires
all of its packages to do something in (arguments), the build system
should be extended :)

This is my goal, and I tried to accomplish this initially, because as
you can see for git checkouts most of the time import-path is url
minus scheme://. But I was unable to retrieve the url in the build
phases. So I did it like this instead, to get something that worked;
also something like #:import-path is required where import-path can't
be derived from the url.

I would need concrete help with this, that is getting the url and
perhaps what (method) was used.

+       #:phases
+       (modify-phases %standard-phases
+         (replace 'delete-files
+           (lambda _
+ (delete-file-recursively "src/github.com/syncthing/syncthing/vendor")))
+
+         (replace 'build
+           (lambda* (#:key inputs #:allow-other-keys)
+ (with-directory-excursion "src/github.com/syncthing/syncthing" + (zero? (system* "go" "run" "build.go" "install" "syncthing" "-no-upgrade")))))
+
+         (replace 'install
+           (lambda _
+ (copy-recursively "src/github.com/syncthing/syncthing/bin/" + (string-append (assoc-ref %outputs "out") "/bin"))
+             (copy-recursively "pkg"
+ (string-append (assoc-ref %outputs "out") "/pkg"))
+             (copy-recursively "src"
+ (string-append (assoc-ref %outputs "out") "/src")))))))
+

Does this package need to use custom build and install phases?

Yes. They use their own build program (build.go) and arguments. And
the resulting binary is put in a non-standard location,
"src/github.com/syncthing/syncthing/bin". If they had put it in "bin/"
overriding this phase wouldn't be necessary.

+(define-public golang-github-com-audriusbutkevicius-go-nat-pmp
+  (let ((commit "452c97607362b2ab5a7839b8d1704f0396b640ca"))

Don't forget the revision counter :)

Oh, I didn't see the need for it, with version being:
"20160522.452c976".

Should version really be: "20160522-1.452c976"? I don't see revision
here contributing any value.

+(define-public golang-github-com-bkaradzic-go-lz4
+  (package
+    (name "golang-github-com-bkaradzic-go-lz4")
+    (version "1.0.0")
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/bkaradzic/go-lz4";)
+ (commit "74ddf82598bc4745b965729e9c6a463bedd33049")))

For packages that we build from a Git tag (rather than an untagged
commit), you should do (commit (string-append "v" version)).

Right. I've attached an updated patch with this.

Although, if there is a release tarball, it's preferable to use it.

Sure, we can use tarballs when this is supported :P

+(define-public golang-github-com-calmh-xdr
+  (let ((commit "f9b9f8f7aa27725f5cabb699bd9099ca7ce09143")
+        (revision "1"))
+    (package
+      (name "golang-github-com-calmh-xdr")
+ (version (string-append "2.0.0-" revision "." (string-take commit 7)))
+      (source (origin
+                (method git-fetch)
+                (uri (git-reference
+                      (url "https://github.com/calmh/xdr";)
+                      (commit commit)))

You could also fetch this one with the Git tag, or use the tarball.

+(define-public golang-github-com-cznic-bufs
+  (let ((commit "3dcccbd7064a1689f9c093a988ea11ac00e21f51"))
+    (package
+      (name "golang-github-com-cznic-bufs")
+      (version (string-append "20160818." (string-take commit 7)))

Is this string '20160818' the date of the commit? If the package has no
releases, we use '0.0.0' in place of the version.

Yes. I figure this is best because some later project we'll package
may be pinned to an earlier commit of a library, and then what?
Incrementing the revision number would suggest this was of newer date,
when it's not, and make the older commit appear as the newest version
of the two to Guix.

Projects not pinned to a library version will then use whatever commit
was added last rather than the newest.

+    (arguments
+     `(#:import-path "github.com/cznic/internal/slice"
+       #:unpack-path "github.com/cznic/internal"))

Is it too much to wonder if both the import-path and the unpack-path
could be auto-generated in cases like this? :)

We should consider this together with the url:
(url (string-append "https://github.com/cznic/internal";))

"slice" is here a directory/package in the repository. For github.com
we could probably assume that the repo is github.com/account/repo, but
other domains may have different layouts. Also keeping in mind that
the requested package could be a subpackage,
"github.com/cznic/internal/slice/tricks".

For github.com this would suggest we use url of the subpackage in the
recipe, otherwise we don't have the import-path, but this url is not
retreivable (404 Not Found). And then we would need for (git-fetch) to
cut the url after the repo part. At this point I think we're at a
place we don't want to be.

I think the best we can do is only to skip #:unpack-path in these
scenarios, and unpack according to domain and path of the url, as long
as domain and path matches the start of the import-path. But as
mentioned earlier, I don't know how to access the url in the build
phases.

I like that you're looking for simplifying the recipes, this is my
goal too. And I'm sure we can automate more.


Thanks for this thorough review! :)

On 2016-12-16 02:52, Leo Famulari wrote:
On Thu, Dec 15, 2016 at 10:06:59PM +0100, Petter wrote:
Hi again,

I've prefixed most of the packages with "golang-" now. However, there are
some
packages already starting with "golang-", f.ex.
"golang-org-x-text-unicode-norm",
and I left those alone. It's inconsistent, but I felt this was preferable to
names like "golang-golang-org-x-text-unicode-norm". What do you think?

That's the right way. It fits the package naming guidelines:

https://www.gnu.org/software/guix/manual/html_node/Package-Naming.html

Finally, there's a telemetry configuration in Syncthing. It is opt-in;
but it will ask the user after a few hours if they want to join. The
plan is to disable the question, however I suspect I've messed up the
build system in that area, so this takes some more looking in to.

Personally, I'm fine with the upstream "opt-in" nag warning. It only
shows up when you open the Syncthing web interface; it's not an
intrusive desktop "notification". Once the user has said "yes" or "no",
it doesn't appear again until Syncthing makes a change to what
information they collect.

But, if people think our package should never ask, I don't mind if we
disable the request, as long as we share our changes with the Syncthing
project and they don't notice anything broken.

Many of my comments below are about tedious things. Let us know if you
are getting sick of working on these packages, and I will help :) This
includes improving the descriptions.

Date: Thu, 15 Dec 2016 21:42:03 +0100
Subject: [PATCH] gnu: Add Syncthing.

* gnu/packages/syncthing.scm: New file.

Cool!

+(define-public syncthing

+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/syncthing/syncthing/";)
+                    (commit (string-append "v" version))))
+ (file-name (string-append name "-" version "-checkout"))

We should use the Syncthing release tarball:

https://github.com/syncthing/syncthing/releases/download/v0.14.14/syncthing-source-v0.14.14.tar.gz

+    (arguments
+     `(#:import-path "github.com/syncthing/syncthing"

What do you think about having the go-build-system try to automatically
generate the import-path based on the source URL, with the option for
the packager to set it manually, as shown here?

For many of the packages in this patch (which will eventually be split
into one package per patch ;) ), that auto-generated import-path
could be correct.

I think that an (arguments) field indicates that the package's build
scripts have deviated from the standard. If a Guix build system requires
all of its packages to do something in (arguments), the build system
should be extended :)

+       #:phases
+       (modify-phases %standard-phases
+         (replace 'delete-files
+           (lambda _
+ (delete-file-recursively "src/github.com/syncthing/syncthing/vendor")))
+
+         (replace 'build
+           (lambda* (#:key inputs #:allow-other-keys)
+ (with-directory-excursion "src/github.com/syncthing/syncthing" + (zero? (system* "go" "run" "build.go" "install" "syncthing" "-no-upgrade")))))
+
+         (replace 'install
+           (lambda _
+ (copy-recursively "src/github.com/syncthing/syncthing/bin/" + (string-append (assoc-ref %outputs "out") "/bin"))
+             (copy-recursively "pkg"
+ (string-append (assoc-ref %outputs "out") "/pkg"))
+             (copy-recursively "src"
+ (string-append (assoc-ref %outputs "out") "/src")))))))
+

Does this package need to use custom build and install phases?

+(define-public golang-github-com-audriusbutkevicius-go-nat-pmp
+  (let ((commit "452c97607362b2ab5a7839b8d1704f0396b640ca"))

Don't forget the revision counter :)

+(define-public golang-github-com-bkaradzic-go-lz4
+  (package
+    (name "golang-github-com-bkaradzic-go-lz4")
+    (version "1.0.0")
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/bkaradzic/go-lz4";)
+ (commit "74ddf82598bc4745b965729e9c6a463bedd33049")))

For packages that we build from a Git tag (rather than an untagged
commit), you should do (commit (string-append "v" version)).

Although, if there is a release tarball, it's preferable to use it.

+(define-public golang-github-com-calmh-xdr
+  (let ((commit "f9b9f8f7aa27725f5cabb699bd9099ca7ce09143")
+        (revision "1"))
+    (package
+      (name "golang-github-com-calmh-xdr")
+ (version (string-append "2.0.0-" revision "." (string-take commit 7)))
+      (source (origin
+                (method git-fetch)
+                (uri (git-reference
+                      (url "https://github.com/calmh/xdr";)
+                      (commit commit)))

You could also fetch this one with the Git tag, or use the tarball.

+(define-public golang-github-com-cznic-bufs
+  (let ((commit "3dcccbd7064a1689f9c093a988ea11ac00e21f51"))
+    (package
+      (name "golang-github-com-cznic-bufs")
+      (version (string-append "20160818." (string-take commit 7)))

Is this string '20160818' the date of the commit? If the package has no
releases, we use '0.0.0' in place of the version.

+    (arguments
+     `(#:import-path "github.com/cznic/internal/slice"
+       #:unpack-path "github.com/cznic/internal"))

Is it too much to wonder if both the import-path and the unpack-path
could be auto-generated in cases like this? :)

Attachment: 0001-gnu-Add-Syncthing.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]