Re: 01/01: gnu: Add rclone.

Re: 01/01: gnu: Add rclone.

[Mark H Weaver]

Hi Nicolas,

guix-commits@gnu.org writes:

> ngz pushed a commit to branch master
> in repository guix.
>
> commit 8b5e2e94afdef6430583c3a0ef02fe2d7fcc16d1
> Author: Nicolas Goaziou <mail@nicolasgoaziou.fr>
> Date:   Sun Nov 25 01:14:28 2018 +0100
>
>     gnu: Add rclone.
>     
>     * gnu/packages/sync.scm (rclone): New variable.

Did you test this?

There's a syntax error in the code, such that it not only fails to
build, but it fails even to produce a _derivation_ describing the build.
Unfortunately, this kind of error in any package causes problems for our
build farm, which currently assumes that this kind of error will not
occur.  Such errors not only cause the individual build to fail, but
moreover for the entire process that generates a new CI "evaluation" to
fail, which effectively prevents subsequent package updates on that
branch from being built until the problem is corrected.

I reverted this commit.

Please be careful when pushing to master.

     Thanks,
       Mark


> diff --git a/gnu/packages/sync.scm b/gnu/packages/sync.scm
> index e0b0af3..22262db 100644
> --- a/gnu/packages/sync.scm
> +++ b/gnu/packages/sync.scm
> @@ -3,6 +3,7 @@
>  ;;; Copyright © 2017 Ricardo Wurmus <rekado@elephly.net>
>  ;;; Copyright © 2018 Tobias Geerinckx-Rice <me@tobias.gr>
>  ;;; Copyright © 2018 Ludovic Courtès <ludo@gnu.org>
> +;;; Copyright © 2018 Nicolas Goaziou <mail@nicolasgoaziou.fr>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -22,6 +23,7 @@
>  (define-module (gnu packages sync)
>    #:use-module ((guix licenses) #:prefix license:)
>    #:use-module (guix build-system cmake)
> +  #:use-module (guix build-system go)
>    #:use-module (guix build-system meson)
>    #:use-module (guix download)
>    #:use-module (guix git-download)
> @@ -243,3 +245,45 @@ over the Internet in an HTTP and CDN friendly way;
>  @item An efficient backup system.
>  @end itemize\n")
>      (license license:lgpl2.1+)))
> +
> +(define-public rclone
> +  (package
> +    (name "rclone")
> +    (version "1.45")
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "https://github.com/ncw/rclone.git")
> +                    (commit (string-append "v" version))))
> +              (file-name (git-file-name name version))
> +              (sha256
> +               (base32
> +                "06xg0ibv9pnrnmabh1kblvxx1pk8h5rmkr9mjbymv497sx3zgz26"))))
> +    (build-system go-build-system)
> +    (arguments
> +     '(#:import-path "github.com/ncw/rclone"
> +       #:install-source? #f

There is a missing "#:phases" keyword here before (modify-phases ...).

> +       (modify-phases %standard-phases
> +         ;; Fix failure during "check" phase: "/homeless-shelter: permission
> +         ;; denied".
> +         (add-after 'unpack 'set-home-directory
> +           (lambda _ (setenv "HOME" "/tmp") #t)))))
> +    (synopsis "@code{rsync} for cloud storage")
> +    (description "@code{Rclone} is a command line program to sync files and
> +directories to and from different cloud storage providers.
> +
> +Features include:
> +@itemize
> +@item MD5/SHA1 hashes checked at all times for file integrity
> +@item Timestamps preserved on files
> +@item Partial syncs supported on a whole file basis
> +@item Copy mode to just copy new/changed files
> +@item Sync (one way) mode to make a directory identical
> +@item Check mode to check for file hash equality
> +@item Can sync to and from network, e.g., two different cloud accounts
> +@item Optional encryption (Crypt)
> +@item Optional cache (Cache)
> +@item Optional FUSE mount (rclone mount)
> +@end itemize")
> +    (home-page "https://rclone.org/")
> +    (license license:expat)))

Re: 01/01: gnu: Add rclone.

[Nicolas Goaziou]

Hello,

Mark H Weaver <mhw@netris.org> writes:

> Did you test this?

Yes, of course I tested it. I was using it for my own needs before
sending the patch for review. I obviously fumbled somehow when I sent
the mail, but I still do not get when or why.

> There's a syntax error in the code, such that it not only fails to
> build, but it fails even to produce a _derivation_ describing the build.
> Unfortunately, this kind of error in any package causes problems for our
> build farm, which currently assumes that this kind of error will not
> occur.  Such errors not only cause the individual build to fail, but
> moreover for the entire process that generates a new CI "evaluation" to
> fail, which effectively prevents subsequent package updates on that
> branch from being built until the problem is corrected.

I suggest to add a check, if possible, for that kind of mistake — e.g.,
check if `arguments' value is a valid property list — because forgetting
a line in a leaf package should not cause the whole build farm to die.

> I reverted this commit.

OK. I'll fix the commit.

> Please be careful when pushing to master.

I sincerely hope you're not suggesting I'm not careful when I send
a patch or push it to master. You must know carefulness is not strictly
equivalent to perfectness. Not quite.

Thank you for fixing my mistake.

Regards,

-- 
Nicolas Goaziou

Re: 01/01: gnu: Add rclone.

[Ludovic Courtès]

Hello,

Nicolas Goaziou <mail@nicolasgoaziou.fr> skribis:

> Mark H Weaver <mhw@netris.org> writes:
>
>> Did you test this?
>
> Yes, of course I tested it. I was using it for my own needs before
> sending the patch for review. I obviously fumbled somehow when I sent
> the mail, but I still do not get when or why.

Yeah, these things happen, and evidently I overlooked it as well when
reviewing the patch.  Mark reverted it quickly enough that this was
mostly harmless.

>> There's a syntax error in the code, such that it not only fails to
>> build, but it fails even to produce a _derivation_ describing the build.
>> Unfortunately, this kind of error in any package causes problems for our
>> build farm, which currently assumes that this kind of error will not
>> occur.  Such errors not only cause the individual build to fail, but
>> moreover for the entire process that generates a new CI "evaluation" to
>> fail, which effectively prevents subsequent package updates on that
>> branch from being built until the problem is corrected.
>
> I suggest to add a check, if possible, for that kind of mistake — e.g.,
> check if `arguments' value is a valid property list — because forgetting
> a line in a leaf package should not cause the whole build farm to die.

Longer-term I think we should all push to a dedicated branch and have
server-side automation apply to ‘master’ patches that pass a few tests
(commit signed by an authorized key, ‘guix lint’, things like that.)

We’ll get there but for now the test is when CI gets around to
evaluating the given commit.

Ludo’.

Re: 01/01: gnu: Add rclone.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 427 bytes --]

On Wed, Nov 28, 2018 at 03:50:14PM +0100, Ludovic Courtès wrote:
> Longer-term I think we should all push to a dedicated branch and have
> server-side automation apply to ‘master’ patches that pass a few tests
> (commit signed by an authorized key, ‘guix lint’, things like that.)

Some Ubuntu people gave a talk on this subject at FOSDEM 2017:

https://archive.fosdem.org/2017/schedule/event/distribution_ci/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]