[Caml-list] Trivial compiler patches

[Caml-list] Trivial compiler patches

[Richard W.M. Jones]

What's a good place to send trivial compiler patches?  I don't want to
open a new bug for what is essentially a one-line aarch64 fix.

Rich.

-- 
Richard Jones
Red Hat

Re: [Caml-list] Trivial compiler patches

[Jacques Garrigue]

On 2014/03/22 07:28, Richard W.M. Jones wrote:
> 
> What's a good place to send trivial compiler patches?  I don't want to
> open a new bug for what is essentially a one-line aarch64 fix.

You should. It would save you the time spent asking this question :-)
More seriously, in theory all bug fixes should be tracked, even trivial
ones, since it lets others know where they were fixed (and where not).

	Jacques

Re: [Caml-list] Trivial compiler patches

[Richard W.M. Jones]

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

On Sat, Mar 22, 2014 at 09:46:53AM +0900, Jacques Garrigue wrote:
> On 2014/03/22 07:28, Richard W.M. Jones wrote:
> > 
> > What's a good place to send trivial compiler patches?  I don't want to
> > open a new bug for what is essentially a one-line aarch64 fix.
> 
> You should. It would save you the time spent asking this question :-)
> More seriously, in theory all bug fixes should be tracked, even trivial
> ones, since it lets others know where they were fixed (and where not).

Let me tell you how ordinary open source projects operate.

(1) They use git.  No one uses svn.

(2) There is usually a mailing list where patches are posted,
discussed and refined.  For example:

https://lists.gnu.org/archive/html/qemu-devel/2014-03/threads.html

(3) There may be another path for trivial fixes to get into the
project, eg. for typos, obvious build fixes, whitespace changes and so
on.  In qemu this is:

http://wiki.qemu.org/Contribute/TrivialPatches
https://lists.nongnu.org/mailman/listinfo/qemu-trivial

This encourages people not to give up on simple fixes because of
excessive process.

(4) They use a decent bug tracking system (ie. not Mantis).

Rich.

-- 
Richard Jones
Red Hat

[-- Attachment #2: 0001-arm64-Trivial-fix-for-missing-parameter.patch --]
[-- Type: text/x-diff, Size: 832 bytes --]

From a2395eadd37ac467c5964d2cbabcbf32e1100110 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Sat, 22 Mar 2014 02:11:32 -0500
Subject: [PATCH] arm64: Trivial fix for missing parameter.

---
 asmcomp/arm64/emit.mlp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/asmcomp/arm64/emit.mlp b/asmcomp/arm64/emit.mlp
index 341812f..274e6ff 100644
--- a/asmcomp/arm64/emit.mlp
+++ b/asmcomp/arm64/emit.mlp
@@ -604,7 +604,7 @@ let emit_instr i =
         `	ldr	{emit_reg reg_trap_ptr}, [sp], 16\n`;
         cfi_adjust_cfa_offset (-16);
         stack_offset := !stack_offset - 16
-    | Lraise ->
+    | Lraise k ->
         begin match !Clflags.debug, k with
         | true, (Lambda.Raise_regular | Lambda.Raise_reraise) ->
           `	bl	{emit_symbol "caml_raise_exn"}\n`;
-- 
1.8.3.1

Re: [Caml-list] Trivial compiler patches

[Gabriel Scherer]

I just created a Mantis issue for the patch:
  http://caml.inria.fr/mantis/view.php?id=6351

The workflow is the following:
  - login; your password should be (random-generated and) recorded in
your browser, two clicks
  - click "report issue", fill relevant info, upload patch, send

It's about two minutes. Alternatively you could have sent a
pull-request against github/ocaml/ocaml (as part of the ongoing "patch
review experiment" http://alan.petitepomme.net/cwn/2014.02.04.html#2
), but I think the workflow is not much quicker (though there are
command-line tools to make it faster).

I looked at the code and indeed the fix itself is trivial, but it was
still useful to make a review: the corresponding arm/emit.mlp change
had additional primitive-registering stuff, and I wondered whether
that also needed to be done in arm64/emit.mlp (answer: no, that is the
only necessary change).

On Sat, Mar 22, 2014 at 8:15 AM, Richard W.M. Jones <rich@annexia.org> wrote:
> On Sat, Mar 22, 2014 at 09:46:53AM +0900, Jacques Garrigue wrote:
>> On 2014/03/22 07:28, Richard W.M. Jones wrote:
>> >
>> > What's a good place to send trivial compiler patches?  I don't want to
>> > open a new bug for what is essentially a one-line aarch64 fix.
>>
>> You should. It would save you the time spent asking this question :-)
>> More seriously, in theory all bug fixes should be tracked, even trivial
>> ones, since it lets others know where they were fixed (and where not).
>
> Let me tell you how ordinary open source projects operate.
>
> (1) They use git.  No one uses svn.
>
> (2) There is usually a mailing list where patches are posted,
> discussed and refined.  For example:
>
> https://lists.gnu.org/archive/html/qemu-devel/2014-03/threads.html
>
> (3) There may be another path for trivial fixes to get into the
> project, eg. for typos, obvious build fixes, whitespace changes and so
> on.  In qemu this is:
>
> http://wiki.qemu.org/Contribute/TrivialPatches
> https://lists.nongnu.org/mailman/listinfo/qemu-trivial
>
> This encourages people not to give up on simple fixes because of
> excessive process.
>
> (4) They use a decent bug tracking system (ie. not Mantis).
>
> Rich.
>
> --
> Richard Jones
> Red Hat
>
> --
> Caml-list mailing list.  Subscription management and archives:
> https://sympa.inria.fr/sympa/arc/caml-list
> Beginner's list: http://groups.yahoo.com/group/ocaml_beginners
> Bug reports: http://caml.inria.fr/bin/caml-bugs

Re: [Caml-list] Trivial compiler patches

[Goswin von Brederlow]

On Sat, Mar 22, 2014 at 09:28:29AM +0100, Gabriel Scherer wrote:
> Alternatively you could have sent a
> pull-request against github/ocaml/ocaml (as part of the ongoing "patch
> review experiment" http://alan.petitepomme.net/cwn/2014.02.04.html#2
> ), but I think the workflow is not much quicker (though there are
> command-line tools to make it faster).

Everyone that has never done this before should take a look. Github
has a "help" link that teaches you various steps and makes even first
time use verry easy.

In short you do the following:

1) Point your browser at github.com, log in (create an account if you
   don't have one) and search for ocaml [1].
2) click "fork" to make your own copy of the ocaml repository
3) clone your own ocaml repository (on your local machine)
4) edit, compile, test
5) commit the changes (on your local machine)
6) push changes back to github (now everyone can see them)
7) back to your browser, update the page for your own ocaml repository
   to see your changes appear
8) click "pull request", review the diff and click "create pull request"
9) wait for the pull request to be accepted

I prefer to make a fresh branch for every pull request and have those
then merged into my master branch that I use locally. But that is up
to you. The github frontend keeps track of your pull requests and
tells you when it got merged and makes it easy to remove the pull
request branches once they are obsolete. The extra branch also makes
it easier to rebase patches in case they need to be updated.

I only recently made my first pull request via github and found it
amazingly simple and comfortable to use.

MfG
	Goswin

PS: you do 1-3 only once, 4-6 all the time as you develope your patch
and 8 once for every patch you want to submitt.

--

[1] https://github.com/ocaml/ocaml

Re: [Caml-list] Trivial compiler patches

[Daniel Bünzli]

Le mardi, 25 mars 2014 à 16:49, Goswin von Brederlow a écrit :

> The extra branch also makes
> it easier to rebase patches in case they need to be updated.

For being able to rebase against upstream you also need to do the following  

https://help.github.com/articles/fork-a-repo#step-3-configure-remotes

which makes the whole procedure quite involved if you just want to submit *once* a trivial patch.  

Best,

Daniel

Re: [Caml-list] Trivial compiler patches

[Richard W.M. Jones]

I have to say that a number of projects I've been involved with have
rejected github pulls as a method of working on patches.  I think the
main reasons are:

 - A mailing list allows you to use your regular editor when
   commenting.

 - A mailing list provides a text-based, open archive of historical
   discussions of patches.

 - github pulls always(?) turn into merge commits, instead of a
   linear history.

 - github is closed source

 - AFAIK github is not integrated into any CI system (compare, say,
   Gerrit + Jenkins -- although Gerrit + Jenkins have their share of
   problems as well).

Now this is not to say that github will not work well for the OCaml
community.

Personally I don't think that patch review is anywhere close to being
a solved problem.  It's an important area requiring much more
research!

Rich.

-- 
Richard Jones
Red Hat

Re: [Caml-list] Trivial compiler patches

[Anil Madhavapeddy]

On 25 Mar 2014, at 16:16, Richard W.M. Jones <rich@annexia.org> wrote:

> I have to say that a number of projects I've been involved with have
> rejected github pulls as a method of working on patches.  I think the
> main reasons are:
> 
> - A mailing list allows you to use your regular editor when
>   commenting.
> 
> - A mailing list provides a text-based, open archive of historical
>   discussions of patches.

These two are definitely useful, although it substantially increases
the amount of incoming email for all developers (especially with 
large, repeated patchsets that are being iterated over).

> - github pulls always(?) turn into merge commits, instead of a
>   linear history.

Only if they're merged via the web interface.  They can be rebased
and pushed directly as usual from the command line.

> - github is closed source

Certainly a concern.

> 
> - AFAIK github is not integrated into any CI system (compare, say,
>   Gerrit + Jenkins -- although Gerrit + Jenkins have their share of
>   problems as well).

GitHub has fantastic CI support via Travis -- easily the best of
anything I've used in the past, and particularly so because it's
free and hosted elsewhere.

See: http://anil.recoil.org/2013/09/30/travis-and-ocaml.html
for an overview of getting started with it.

In general, the GitHub API support makes it sufficiently easy to
hook in third-party workflows that the OCaml GitHub mirror will
continue to be useful even if Gabriel's experiment with pull requests
doesn't work out.

> Personally I don't think that patch review is anywhere close to being
> a solved problem.  It's an important area requiring much more
> research!

I spent a while trying these out for Real World OCaml, and noted
that all of the existing tools only support *patch* review, but not
general *code* review.  Consider, for example, someone reading a
book chapter, or the 4.01.0 x86_64 code generator and wanting to leave
comments on the whole tree, not just on a specific isolated patch.

An odd omission in the space, or I just missed an area of tooling
that deals with this...

-anil

Re: [Caml-list] Trivial compiler patches

[Goswin von Brederlow]

On Tue, Mar 25, 2014 at 04:16:48PM +0000, Richard W.M. Jones wrote:
> I have to say that a number of projects I've been involved with have
> rejected github pulls as a method of working on patches.  I think the
> main reasons are:
> 
>  - A mailing list allows you to use your regular editor when
>    commenting.
> 
>  - A mailing list provides a text-based, open archive of historical
>    discussions of patches.

And makes it hard to follow or later trace the developement due to the
amount of intermixed mails.

Rebasing a patch set for every change also makes it hard to follow
since rebase destroys history. So far revision control systems lack
that extra dimension needed to track the developement of a patch set
over time.

So overall there is no ideal solution yet. As you say below it is
still an unsolved problem.

>  - github pulls always(?) turn into merge commits, instead of a
>    linear history.

That happens when you merge pull requests via the web interface. You
don't have to.

Also does that realy hold true if the pull request is based on the
HEAD of the main branch and only has one commit (only one patch in the
set)? That would create a merge with 2 parents that are both the same.
 
>  - github is closed source

I don't mind. It is free as in beer, it works and it doesn't poison
your project (no lock in). You can always just use plain git at any
time, which is free.
 
>  - AFAIK github is not integrated into any CI system (compare, say,
>    Gerrit + Jenkins -- although Gerrit + Jenkins have their share of
>    problems as well).
> 
> Now this is not to say that github will not work well for the OCaml
> community.
> 
> Personally I don't think that patch review is anywhere close to being
> a solved problem.  It's an important area requiring much more
> research!
> 
> Rich.

Maybe once it is solved someone will implement a nice free platform
for it. Till then lets see what other people come up with, free or not.

MfG
	Goswin

Re: [Caml-list] Trivial compiler patches

[Anil Madhavapeddy]

On 27 Mar 2014, at 09:34, Goswin von Brederlow <goswin-v-b@web.de> wrote:

>> 
>> - github is closed source
> 
> I don't mind. It is free as in beer, it works and it doesn't poison
> your project (no lock in). You can always just use plain git at any
> time, which is free.

There's a significant amount of metadata that isn't track in Git.
Issues, pull requests, releases, comments, descriptions are all
available via the API, but not in Git.  Thomas Gazagnaire started
a CLI that dumps that information into a local git repository using
his ocaml-git implementation, but it's not quite complete yet. Do
get in touch if anyone's interested in helping finishing that effort
off to provide a GitHub export to a more neutral format...

-anil

Re: [Caml-list] Trivial compiler patches

[Thomas Gazagnaire]

> Thomas Gazagnaire started
> a CLI that dumps that information into a local git repository using
> his ocaml-git implementation, but it's not quite complete yet. Do
> get in touch if anyone's interested in helping finishing that effort
> off to provide a GitHub export to a more neutral format...

The repo is there: https://github.com/samoht/ghim for whose interested. But that's just an early experiment for now on. Patches are more than welcome!

Thomas

Re: [Caml-list] Trivial compiler patches

[François Bobot]

On 27/03/2014 10:34, Goswin von Brederlow wrote:
> On Tue, Mar 25, 2014 at 04:16:48PM +0000, Richard W.M. Jones wrote:
>> I have to say that a number of projects I've been involved with have
>> rejected github pulls as a method of working on patches.  I think the
>> main reasons are:
>>
>>  - A mailing list allows you to use your regular editor when
>>    commenting.
>>
>>  - A mailing list provides a text-based, open archive of historical
>>    discussions of patches.
> 
> And makes it hard to follow or later trace the developement due to the
> amount of intermixed mails.
> 
> Rebasing a patch set for every change also makes it hard to follow
> since rebase destroys history. So far revision control systems lack
> that extra dimension needed to track the developement of a patch set
> over time.

One way is to not rebase during the review process, add the modification on top and rebase at the
end of the review process. The autosquashing feature of git makes that quite simple, "git commit
[--fixup|--squash]", since that allows to record which commit is fixed. You can look at
https://github.com/ocaml/ocaml/pull/22 for an idea (the commit are prefixed with "fixup!").

-- 
François