diff mbox series

[bug#40764] New package: r-restrserve

Message ID 87pnbzpyeq.fsf@ericcbrown.com
State Accepted
Headers show
Series [bug#40764] New package: r-restrserve | expand

Checks

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

Commit Message

Eric Brown April 22, 2020, 11:12 a.m. UTC
Dear Guix,

Please see attached diff for r-restrserve.  Please note that it depends
on a patch r-rserve which I submitted just a moment ago.

Cheers,
Eric

Comments

Marius Bakke May 30, 2020, 5:56 p.m. UTC | #1
Eric Brown <ecbrown@ericcbrown.com> writes:

> Dear Guix,
>
> Please see attached diff for r-restrserve.  Please note that it depends
> on a patch r-rserve which I submitted just a moment ago.

This patch no longer applies.  Can you rebase it on 'master'?  Thanks in
advance, and sorry for the delay.

Also, make sure 'guix lint r-restrserve' passes.  :-)
Christopher Baines Dec. 9, 2020, 7:36 p.m. UTC | #2
Eric Brown <ecbrown@ericcbrown.com> writes:

> Dear Guix,
>
> Please see attached diff for r-restrserve.  Please note that it depends
> on a patch r-rserve which I submitted just a moment ago.

Hi Eric,

I've gone ahead and pushed this as
e36291ef52a30b1c667b78ef76c1980363f8c138.

Due to the delay in merging this, I updated the package to 0.4.0 and
added some more inputs.

In the future, I'd strongly recommend not adding packages to the bottom
of modules, unless you really want the package definition to be
there. If every new definition gets added at the bottom, merge conflicts
become very likely. Related to this, I also moved the package definition
up off the bottom of the module.

Thanks,

Chris
Simon Tournier Dec. 10, 2020, 12:36 p.m. UTC | #3
Hi Chris,

On Wed, 09 Dec 2020 at 19:36, Christopher Baines <mail@cbaines.net> wrote:

> In the future, I'd strongly recommend not adding packages to the bottom
> of modules, unless you really want the package definition to be
> there. If every new definition gets added at the bottom, merge conflicts
> become very likely. Related to this, I also moved the package definition
> up off the bottom of the module.

What do you mean?  From my understanding, we always add new R packages
at the botton of the files cran.scm or bioconductor.scm; mainly.

Instead, what do you suggest?  Again, from my understanding, merge
conflicts could happen wherever the package is added.  If the package is
added at one location in the file and this location does not make sense
anymore because other packages had been added before this location, then
conflict.  For sure, adding bottom increases the probability it
happens. ;-) But adding elsewhere does not prevent neither.

The only fix to avoid boring conflicts is to reduce the time between the
submission and the merge, IMHO.

Do I miss something?

All the best,
simon
Ricardo Wurmus Dec. 10, 2020, 1:18 p.m. UTC | #4
zimoun <zimon.toutoune@gmail.com> writes:

> Hi Chris,
>
> On Wed, 09 Dec 2020 at 19:36, Christopher Baines <mail@cbaines.net> wrote:
>
>> In the future, I'd strongly recommend not adding packages to the bottom
>> of modules, unless you really want the package definition to be
>> there. If every new definition gets added at the bottom, merge conflicts
>> become very likely. Related to this, I also moved the package definition
>> up off the bottom of the module.
>
> What do you mean?  From my understanding, we always add new R packages
> at the botton of the files cran.scm or bioconductor.scm; mainly.

The problem is 100% with tooling.  Git needs some context to apply
patches.  When you add package definitions to the bottom and that bottom
context keeps changing you will always need to tell Git how to apply that
patch.

By picking an arbitrary location somewhere in the file you avoid this
problem because it’s unlikely that other people will pick that very same
location.

> The only fix to avoid boring conflicts is to reduce the time between the
> submission and the merge, IMHO.

Not even that would be a fix, because you can have two different people
submitting patches for modifications at the bottom of the file at the
same time.  Applying them one after the other will result in the same
problem, no matter how fast we are.

Of course it’s always better to reduce time between submission and
application.

It would be *great* if we could find another way to appease git and do
the right thing for the most common case of simply adding a package
definition to the bottom of the file, no matter what context there might
be right above.
Efraim Flashner Dec. 10, 2020, 2:22 p.m. UTC | #5
On Thu, Dec 10, 2020 at 02:18:27PM +0100, Ricardo Wurmus wrote:
> 
> zimoun <zimon.toutoune@gmail.com> writes:
> 
> > Hi Chris,
> >
> > On Wed, 09 Dec 2020 at 19:36, Christopher Baines <mail@cbaines.net> wrote:
> >
> >> In the future, I'd strongly recommend not adding packages to the bottom
> >> of modules, unless you really want the package definition to be
> >> there. If every new definition gets added at the bottom, merge conflicts
> >> become very likely. Related to this, I also moved the package definition
> >> up off the bottom of the module.
> >
> > What do you mean?  From my understanding, we always add new R packages
> > at the botton of the files cran.scm or bioconductor.scm; mainly.
> 
> The problem is 100% with tooling.  Git needs some context to apply
> patches.  When you add package definitions to the bottom and that bottom
> context keeps changing you will always need to tell Git how to apply that
> patch.
> 
> By picking an arbitrary location somewhere in the file you avoid this
> problem because it’s unlikely that other people will pick that very same
> location.
> 
> > The only fix to avoid boring conflicts is to reduce the time between the
> > submission and the merge, IMHO.
> 
> Not even that would be a fix, because you can have two different people
> submitting patches for modifications at the bottom of the file at the
> same time.  Applying them one after the other will result in the same
> problem, no matter how fast we are.
> 
> Of course it’s always better to reduce time between submission and
> application.
> 
> It would be *great* if we could find another way to appease git and do
> the right thing for the most common case of simply adding a package
> definition to the bottom of the file, no matter what context there might
> be right above.
> 

This is why I like sorting them alphabetically :)
Simon Tournier Dec. 10, 2020, 2:25 p.m. UTC | #6
Hi Ricardo,

On Thu, 10 Dec 2020 at 14:18, Ricardo Wurmus <rekado@elephly.net> wrote:
>> On Wed, 09 Dec 2020 at 19:36, Christopher Baines <mail@cbaines.net> wrote:
>>
>>> In the future, I'd strongly recommend not adding packages to the bottom
>>> of modules, unless you really want the package definition to be
>>> there. If every new definition gets added at the bottom, merge conflicts
>>> become very likely. Related to this, I also moved the package definition
>>> up off the bottom of the module.
>>
>> What do you mean?  From my understanding, we always add new R packages
>> at the botton of the files cran.scm or bioconductor.scm; mainly.
>
> The problem is 100% with tooling.  Git needs some context to apply
> patches.  When you add package definitions to the bottom and that bottom
> context keeps changing you will always need to tell Git how to apply that
> patch.

[...]

> It would be *great* if we could find another way to appease git and do
> the right thing for the most common case of simply adding a package
> definition to the bottom of the file, no matter what context there might
> be right above.

The option “format-patch --base” reduces the burden and I proposed this
patch:

   <http://issues.guix.gnu.org/issue/43946>

Adding directly in the patch the (base) commit where the patch is known
to apply fixes most of the issues.  IMO.


All the best,
simon
Oleg Pykhalov Dec. 14, 2020, 9:54 a.m. UTC | #7
Hello,

zimoun <zimon.toutoune@gmail.com> writes:

[…]

> The option “format-patch --base” reduces the burden and I proposed this
> patch:
>
>    <http://issues.guix.gnu.org/issue/43946>
>
> Adding directly in the patch the (base) commit where the patch is known
> to apply fixes most of the issues.  IMO.

It adds ‘base-commit’ and optionally ‘prerequisite-patch-id’ (see [1]), which will
help you to checkout base-commit and apply a patch, but still you will
need to rebase on origin/master with solving conflicts AFAIK.

Also, on git version 2.29.2 --base requires an argument.  If we apply
the patch on 43946 issue, we should mention this in example.  

And, IMHO, the --base flag should be mention inside the first paragraph
of [2], where format-patch is recommended for patch submission.
Otherwise people will send with and without --base, which probably
will not hurt, but still.

[1]: --base=HEAD~~ output example
--8<---------------cut here---------------start------------->8---
oleg@guixsd ~/src/guix-master$ git format-patch --base=HEAD~~ --stdout HEAD~..HEAD
From ea50795d7f54582d5c439f60e836ff2ee5d8aed2 Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Sat, 12 Dec 2020 00:39:26 -0500
Subject: [PATCH] gnu: linux-libre 4.4: Update to 4.4.248.

* gnu/packages/linux.scm (linux-libre-4.4-version): Update to 4.4.248.
(linux-libre-4.4-pristine-source): Update hash.
---
 gnu/packages/linux.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 4c5917bc59..73dec7d1b7 100644
[…]


base-commit: 4c327e025277dd59b864200d5ecf671b5656e7c9
prerequisite-patch-id: 2f8bef97c220d126e922b526c4fccc847592311d
Simon Tournier Dec. 15, 2020, 2:35 p.m. UTC | #8
Hi,

On Mon, 14 Dec 2020 at 12:54, Oleg Pykhalov <go.wigust@gmail.com> wrote:

> It adds ‘base-commit’ and optionally ‘prerequisite-patch-id’ (see [1]), which will
> help you to checkout base-commit and apply a patch, but still you will
> need to rebase on origin/master with solving conflicts AFAIK.

“with **hypothetically** solving conflicts”.  Well, it is by-design
because of Git.

It is still better than the current situation IMHO where it is boring to
apply old patches.


> Also, on git version 2.29.2 --base requires an argument.  If we apply
> the patch on 43946 issue, we should mention this in example.

Agree.

> And, IMHO, the --base flag should be mention inside the first paragraph
> of [2], where format-patch is recommended for patch submission.
> Otherwise people will send with and without --base, which probably
> will not hurt, but still.

Yes, the first paragraph seems better.

More people will send with base-commit and easier automation will be.
For example, we could imagine that the testing build-farm builds the
patch applied at the base-commit, so the reviewer can use the substitute
instead of burning CPU (sometimes the test suite is really resource
hungry).  The reviewer checks the compliance and rebase if needed.


All the best,
simon
Eric Brown May 26, 2021, 10:33 p.m. UTC | #9
On Wed, Apr 22, 2020, at 6:12 AM, Eric Brown wrote:
> Dear Guix,
> 
> Please see attached diff for r-restrserve.  Please note that it depends
> on a patch r-rserve which I submitted just a moment ago.
> 
> Cheers,
> Eric
> 
> 
> Attachments:
> * 0001-gnu-Add-r-restrserve.patch

THanks all for the patch, and advice.  I wasn't sure what to do with new symbols.  Next time I'll lean toward inserting them in the vast middle.  :-)

Best regards,
Eric
diff mbox series

Patch

From 77b08891ad2ffba228858dc0d06b0000b544597a Mon Sep 17 00:00:00 2001
From: Eric Brown <ecbrown@ericcbrown.com>
Date: Wed, 22 Apr 2020 06:07:14 -0500
Subject: [PATCH] gnu: Add r-restrserve.

* gnu/packages/cran.scm (r-restrserve): New variable.
---
 gnu/packages/cran.scm | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/gnu/packages/cran.scm b/gnu/packages/cran.scm
index 70cb7cc700..8b27279f2b 100644
--- a/gnu/packages/cran.scm
+++ b/gnu/packages/cran.scm
@@ -21162,3 +21162,25 @@  evaluated interactively.")
 Bayes factors, posterior model probabilities, and normalizing constants in
 general, via different versions of bridge sampling.")
     (license license:gpl2+)))
+
+(define-public r-restrserve
+  (package
+    (name "r-restrserve")
+    (version "0.2.2")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (cran-uri "RestRserve" version))
+       (sha256
+        (base32 "1b8wbar98qhhl46s4i7qks5nm2wy5bvfi9029gpd4gmqsq4bmbm7"))))
+    (build-system r-build-system)
+    (propagated-inputs
+     `(("r-rserve" ,r-rserve)))
+    (home-page "https://restrserve.org")
+    (synopsis "R web API framework")
+    (description
+     "RestRserve is an R web API framework for building high-performance AND
+robust microservices and app backends. With Rserve backend on UNIX-like systems
+it is parallel by design. It will handle incoming requests in parallel - each
+request in a separate fork.")
+    (license license:gpl2+)))
-- 
2.26.2