R devel - Jan 2015 - [PATCH] Makefile: add support for git svn clones

On 19/01/2015 4:13 PM, Nathan Kurz wrote:
> On Mon, Jan 19, 2015 at 1:00 PM, Felipe Balbi <balbi at kernel.org>
wrote:
>> I just thought that such a small patch which causes no visible change
to
>> SVN users and allow for git users to build R would be acceptable, but
if
>> it isn't, that's fine too.
> 
> Felipe ---
> 
> It would appear that you are unaware that you are walking a minefield
> of entrenched positions and personality conflicts.  For those like
> myself who are mystified by the positions taken in this thread, a
> partial back story may be helpful.
I don't think there should be anything particularly mystifying here.
The people who would have to maintain the patch can't test it.  The
people who can test it can apply it themselves.  It's just a matter of
efficiency.  There's a very easy way for you to use the svn checkout,
and that is to check it out using svn.  If you want to use a tool that's
never tested, then you're on your own.

In another reply to you, Joshua pointed out one of the problems that
using untested tools causes.  It wasted a lot of Hin-Tak's and other
people's time.  That time could have been spent in a productive way, but
it wasn't.

Duncan Murdoch

> 
> In 2012, Han-Tak Leung reported a problem compiling the development
> version of R that he had checked out using git's svn compability
> feature: https://stat.ethz.ch/pipermail/r-devel/2012-October/065133.html
> 
> In 2013, Brian Ripley applied a patch with the comment "trap HK Leung
> misuse" explicitly to prevent users from being able to do this:
>
https://github.com/wch/r-source/commit/4f13e5325dfbcb9fc8f55fc6027af9ae9c7750a3
> 
> Shortly thereafter, Han-Tak tried to start discussion on this list
> about that patch, suggesting that preventing the use of non-SVN
> mirrors reduced the frequency with which development versions would be
> tested:
> https://stat.ethz.ch/pipermail/r-devel/2013-March/066128.html
> 
> The opinions expressed on the thread were universally against Leung.
> Peter Dalgaard summarized as:
> "The generic point is that you are given access to a working tool that
> is internal to the core R developers. We are not putting restrictions
> on what you do with that access, but if you want to play the game by
> other rules than we do, you need to take the consequences. If things
> don't work and you start complaining about them being
"broken", steps
> may be taken to make it clearer who broke them."
> https://stat.ethz.ch/pipermail/r-devel/2013-March/066131.html
> 
> As a newcomer hoping to contribute to R who had already encountered
> this same compilation issue and considered it was a bug, I am
> astounded to learn that it is instead desired and intentional
> behavior.
> 
> --nate
>

On 19 January 2015 at 17:11, Duncan Murdoch wrote:
| The people who would have to maintain the patch can't test it.  

I don't understand this.

The patch, as we may want to recall, was all of

   +GIT := $(shell if [ -d "$(top_builddir)/.git" ]; then \
   +	echo "git"; fi)
   +

and

   -	  (cd $(srcdir); LC_ALL=C TZ=GMT svn info || $(ECHO) "Revision:
-99") 2> /dev/null \
   +	  (cd $(srcdir); LC_ALL=C TZ=GMT $(GIT) svn info || $(ECHO) "Revision:
-99") 2> /dev/null \

I believe you can test that builds works before applying the patch, and
afterwards---even when you do not have git, or in this case a git checkout.
The idiom of expanding a variable to "nothing" if not set is used all
over
the R sources and can be assumed common.  And if (hypothetically speaking)
the build failed when a .git directory was present?  None of R Core's
concern
either as git was never supported.

I really do not understand the excitement over this.  The patch is short,
clean, simple, and removes an entirely unnecessary element of friction.

Dirk

-- 
http://dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org

Fellipe,

CXXR development has moved to github, and we haven't fixed up the build for
using git yet.  Could you send a pull request with your change to the repo
at https://github.com/cxxr-devel/cxxr/?

Also, this patch may be useful for pqR too.
https://github.com/radfordneal/pqR

Thanks

On Mon, Jan 19, 2015 at 2:35 PM, Dirk Eddelbuettel <edd at debian.org>
wrote:
>
> On 19 January 2015 at 17:11, Duncan Murdoch wrote:
> | The people who would have to maintain the patch can't test it.
>
> I don't understand this.
>
> The patch, as we may want to recall, was all of
>
>    +GIT := $(shell if [ -d "$(top_builddir)/.git" ]; then \
>    +    echo "git"; fi)
>    +
>
> and
>
>    -      (cd $(srcdir); LC_ALL=C TZ=GMT svn info || $(ECHO)
"Revision:
> -99") 2> /dev/null \
>    +      (cd $(srcdir); LC_ALL=C TZ=GMT $(GIT) svn info || $(ECHO)
> "Revision: -99") 2> /dev/null \
>
> I believe you can test that builds works before applying the patch, and
> afterwards---even when you do not have git, or in this case a git checkout.
> The idiom of expanding a variable to "nothing" if not set is used
all over
> the R sources and can be assumed common.  And if (hypothetically speaking)
> the build failed when a .git directory was present?  None of R Core's
> concern
> either as git was never supported.
>
> I really do not understand the excitement over this.  The patch is short,
> clean, simple, and removes an entirely unnecessary element of friction.
>
> Dirk
>
> --
> http://dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org
>
> ______________________________________________
> R-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>
	[[alternative HTML version deleted]]

>>>>> Dirk Eddelbuettel <edd at debian.org>
>>>>>     on Mon, 19 Jan 2015 16:35:31 -0600 writes:
    > On 19 January 2015 at 17:11, Duncan Murdoch wrote:
    > | The people who would have to maintain the patch can't test it.  

I agree that this is good reason to not add delicate code to the
R sources, indeed,  however, read on ..

    > I don't understand this.

    > The patch, as we may want to recall, was all of

    > +GIT := $(shell if [ -d "$(top_builddir)/.git" ]; then \
    > +	echo "git"; fi)
    > +

    > and

    > -	  (cd $(srcdir); LC_ALL=C TZ=GMT svn info || $(ECHO) "Revision:
-99") 2> /dev/null \
    > +	  (cd $(srcdir); LC_ALL=C TZ=GMT $(GIT) svn info || $(ECHO)
"Revision: -99") 2> /dev/null \

the other thing needed -- apart from a similar patch to the Windows Makefile
is to make the above "portable" in the sense that it works for
standard make as opposed to just GNU make....
This requirement also helps portability to (albeit rare I think) platforms.
AFAICS, this simply needs replacement of  
	$(shell  <stuff>)
by      ` <stuff> `

    > I believe you can test that builds works before applying the patch, and
    > afterwards---even when you do not have git, or in this case a git
checkout.

    > The idiom of expanding a variable to "nothing" if not set is
used all over
    > the R sources and can be assumed common.  And if (hypothetically
speaking)
    > the build failed when a .git directory was present?  None of R
Core's concern
    > either as git was never supported.

    > I really do not understand the excitement over this.  The patch is
short,
    > clean, simple, and removes an entirely unnecessary element of friction.

I agree partly - as I agree with Duncan's points -
and for this case, in spite of good reasons why such a patch can
be problematic to accept, I'm tending to think of  "sponsoring"
it, i.e., putting
it in *and* maintain it (if that maintenance is close to "nil" :-),
notably after you add the changes mentioned above.  

As, indeed, I don't see how it can harm, and you, the git users
among us (= "the people interested in tracking R development
sources") would keep some responsibility with it.

I would like to keep us in a good "community" spirit of
collaborating and focused on the advancement of Free Software in
general and of R in particular, and so help each other as much
as possible with the limited resources we have....

Best,
Martin