From mboxrd@z Thu Jan 1 00:00:00 1970 From: Leo Famulari Subject: Re: gnu: Add syncthing. Date: Thu, 15 Dec 2016 20:52:22 -0500 Message-ID: <20161216015222.GA23115@jasmine> References: <2d54ddb40494e3c6229e2a464ffe02c3@mykolab.ch> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:37955) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cHhhI-0004fJ-PQ for guix-devel@gnu.org; Thu, 15 Dec 2016 20:52:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cHhhF-0007dw-Ky for guix-devel@gnu.org; Thu, 15 Dec 2016 20:52:32 -0500 Received: from out3-smtp.messagingengine.com ([66.111.4.27]:39136) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cHhhF-0007cs-E3 for guix-devel@gnu.org; Thu, 15 Dec 2016 20:52:29 -0500 Content-Disposition: inline In-Reply-To: <2d54ddb40494e3c6229e2a464ffe02c3@mykolab.ch> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Petter Cc: guix-devel@gnu.org 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? :)