git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [GSoC] Convert “git stash” to builtin proposal
@ 2018-03-20 20:09 Paul-Sebastian Ungureanu
  2018-03-20 22:08 ` Christian Couder
  0 siblings, 1 reply; 11+ messages in thread
From: Paul-Sebastian Ungureanu @ 2018-03-20 20:09 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Johannes.Schindelin

Hello,

I have completed the first draft of my proposal for the Google Summer
of Code, which can be found at [1] or below for those who prefer the
text version.

Any feedback is greatly appreciated. Thanks!

[1]
https://docs.google.com/document/d/1TtdD7zgUsEOszHG5HX1at4zHMDsPmSBYWqy
dOPTTpV8/edit?usp=sharing

Best regards,
Paul-Sebastian Ungureanu

# Convert “git stash” to builtin

# Name and Contact Information
Hello! My name is Paul-Sebastian Ungureanu. I am currently a first year
Computer Science & Engineering student at Politehnica University of
Bucharest, Romania.

My email address is ungureanupaulsebastian@gmail.com and my phone
number is [CENSORED]. You can also find me on #git IRC channel as
ungps.

FLOSS manual recommends students to include in their proposal their
postal address and mention a relative as a emergency contact. My
permanent address is [CENSORED]. In case of an emergency, you may
contact my brother, [CENSORED] by email at [CENSORED] or by phone at
[CENSORED].

# Synopsis
Currently, many components of Git are still in the form of Shell or
Perl scripts. This choice makes sense especially when considering how
faster new features can be implemented in Shell and Perl scripts,
rather than writing them in C. However, in production, where Git runs
daily on millions of computers with distinct configurations (i.e.
different operating systems) some problems appear (i.e. POSIX-to-
Windows path conversion issues).

The idea of this project is to take “git-stash.sh” and reimplement it
in C. Apart from fixing compatibility issues and increasing the
performance by going one step closer to native code, I believe this is
an excellent excuse to fix long-standing bugs or implement new minor
features.

# Benefits to community
The essential benefit brought by rewriting Git commands is the
increased compatibility with a large number computers with distinct
configuration. I believe that this project can have a positive impact
on a large mass of developers around the world. By rewriting the code
behind some popular commands and making them “built-in”, developers
will have a better overall experience when using Git and get to focus
on what really matters to them.

As a side effect, there will be a number of other improvements:
increased performance, ability to rethink the design of some code that
suffered from patching along the time, have the chance to create new
features and fix old bugs.

In theory, switching from Bash or Shell scripts, Git will be one step
closer to native code which should have a positive impact on the
performance. Being able to start from a clean slate is a great
opportunity to rethink old designs that may have been patched a lot
during their lifetime. This way, with the help of my mentors, I can
think of new ways to try and remove some limitations of the current
system (if there are any).

Moreover, I believe that the community will benefit greatly from new
features and bug fixes that I could help with. Even though this is not
really one of the main goals of this project, I believe that it would
be easier to fix bugs or implement new features while rewriting the
code. However, I will have to discuss with my mentors and carefully
review issues as I would not want to divagate from the purpose of the
project.

As a last point, I believe it is good to have a more homogenous
codebase, where the majority of the code would be written in C. This
could increase the number of contributions to the project as there are
maybe more programmers who are familiar with C, and not so much with
Perl or shell scripting.

# Deliverables
Deliverable of this project is “git stash” completely rewritten in
portable C code. Along with the new code, there will be some additions
and changes brought to tests to cover any new behaviour.

# Related work
Looking over the list of the other proposed projects, I believe that
“Convert interactive rebase to C” and “git bisect improvements” are the
most alike with this project and may be stretch goal of this project.

Moreover, there is a chance that other scripts could benefit from this
project if it were to be taken as an example for future conversions.

# Biographical information
I am a freshman at Politehnica University of Bucharest, which is
considered to be one of the most prestigious universities in the
Eastern Europe. I consider myself an ambitious software engineer that
enjoys competition. This has been proven by my participation to
programming competitions and extracurricular projects. As much as I
like competitions, I also love working with and meeting people that
share my interests for programming and technology.

Even though, in the last two years I found myself to be more interested
in Android software development rather than competitive programming, I
still take part in most of the competitions, such as contests organized
by Google HashCode, Google KickStart and ACM.

I have a good grasp of programming languages such as C and C++ and I
consider both of them to be my favorites. In the past, I intensively
used shell scripting and Git for some of my personal projects.

One of the facts that motivates me the most is that I get the chance to
improve my abilities by getting the code reviewed by professionals that
have been in the industry for a long period of time. This way, I get
first-hand experience regarding software management and increase my
programming skills in a professional environment.

Ultimately, I believe that being part of a community such as Git would
be a great confidence boost for me knowing that I gave something back
to an excellent piece of software that I am using on a daily basis.

# Project Details
* Plan for converting each command

* Understand the command well: read documentation, discover any
features I were not aware of, find out how it works and why it works
that way.

* Find out all edge and corner cases: these should be quite obvious as
soon as I have a good understanding of how the command works.

* Find out the history of the command and what future improvements can
be made: history of the command will help me know what issues affected
the command, how those issues were fixed and will help me avoid them in
the future. As I mentioned before, I believe that reimplementing those
commands is a great excuse to fix bugs and if there are any, I need to
know about them.

* Convert function: this step is basically makes up the goal of this
project.

* Ensure that no regression occurred: considering that there are plenty
of tests and that I have a good understanding of the function, this
should be a trivial task.

* Finally write more tests to ensure best code coverage.

# Potential difficulties during conversion
The rewritten code may introduce new bugs that may be delaying the
project. I believe there is a small chance this will happen as long as
I spend some time first to fully understand how it works and why it
works that way. Considering the amount of documentation already
available and the discussions from the mailing list, this should be a
trivial task. Moreover, I am already familiar with “git stash”.

There may be old bugs, from the Shell scripts that would cause issues
while porting because they would probably require a discussion about
what implications the bug has and what is the best way to fix it.

Most of the times, Shell scripts tend to be a lot more smaller and hide
a lot of complexity. Moreover, new data structures or utility functions
may be needed, some which exist and need changes or do not exist and
need to be implemented from scratch.

# Wrapping-up the project
In the end, the old Shell script will serve only as a called to the
stash-helper I would be implementing. The final step is to remove the
Shell script and promote the stash-helper to stash builtin.

# Useful resources
There has been a lot of progress made in this direction already and I
believe it will serve of great help. However, from my understanding it
is not yet mergeable and it requires changes.

https://public-inbox.org/git/20170608005535.13080-1-joel@teichroeb.net/
T/#m8849c7ce0ad8516cc206dd6910b79591bf9b3acd

# Project Schedule
I can spend about 25 hours during the exam period (last half of May and
the first half of June) and about 40 hours a week as soon as I am done
with exams. I hope that by the start of the program (14th of May), I
will have completed a good part of the project and that will compensate
for the exam period.

After Google Summer of Code ends, I will do my best to keep
contributing to Git. Hopefully, all the work I proposed over the summer
will be done and I will be able to move towards other areas, maybe by
continuing converting other scripts to builtins or something else,
depending on what are the priorities of the Git team.

Google Summer of Code starts on 14th of May and lasts 13 weeks. I
propose to convert one command per week. I actually do not expect each
command to take exactly one week, some may be easier to convert and
other may be more challenging, but I believe that the average time of
conversion is around 1 week.

I am expecting to submit a patch for every command that is converted.
This way, I have well set milestones and results will be seen as soon
as possible. Moreover, seeing my contributions getting merged will be a
huge confidence boost to myself.

Each “convert” phase of the project below is not only about converting
from Shell to C, but also ensuring that everything is documented, there
are enough tests and there are no regressions.

14th May - 20th May     	- Convert "git stash list"
21st May - 27th May     	- Convert "git stash show"
28th May - 3rd June     	- Convert "git stash save"
4th June - 10th June    	- Convert "git stash push"
11th June - 17th June*  	- Convert "git stash apply"
18th June - 24th June   	- Convert "git stash clear"
25th June - 1st July    	- Convert "git stash create"
2nd July - 8th July     	- Convert "git stash store"
9th July - 15th July*  		- Convert "git stash drop"
16th July - 22nd July    	- Convert "git stash pop"
23rd July - 29th July      	- Convert "git stash branch"
30th July - 5th August 		- Remove old Shell script.
6th August - 12th August* 	- Wrap-up the project by writing more
tests and documentation.
12th August - forever 		- Pick up another project.

* 1st, 2nd and 3rd mentor and student evaluation

# Git contributions
My first contribution to Git was to help making “git tag --contains
<id>” les chatty if <id> is invalid. Looking over the list of available
microprojects, there were a couple of microprojects which got my
attention, but I picked this up because it seemed to be a long-standing 
bug (I noticed it was approached by students in 2016, 2017 and now in
2018). Thanks to the code reviews from the mailing list, after a few
iterations I succeeded in coming up with a patch that not only fixed
the mentioned problem, but also a few others that were having the same
cause.

It got merged in the proposed updates branch.

I worked on only one project because I tried to achieve the best
quality I could. Even after I submitted one patch that was good enough
[7], I decided to try another approach that worked better in the end
[8].

First of all, I weighed the ideas mentioned at [9] and considered the
second one to be the best. After a few iterations [1], [2], [3], [4],
[5] and after Junio’s review I decided to try another approach and
submitted another patch [6].

I changed the approach, because this issue was affecting not only “git
tag” command, but also “git for-each-ref”, “git branch” and many more.
The new submission was based on Junio’s ideea and fixed the cause at a
lower level than the previous patch.

In order to make sure that the new code was 100% correct and
familiarize myself with the testing system, I also write a new set of
tests.

[1]
https://public-inbox.org/git/20180219212130.4217-1-ungureanupaulsebasti
an@gmail.com/

[2]
https://public-inbox.org/git/20180223162557.31477-1-ungureanupaulsebast
ian@gmail.com/

[3]
https://public-inbox.org/git/20180303210938.32474-1-ungureanupaulsebast
ian@gmail.com/

[4]
https://public-inbox.org/git/20180306193116.23876-1-ungureanupaulsebast
ian@gmail.com/

[5]
https://public-inbox.org/git/20180319185436.14309-1-ungureanupaulsebast
ian@gmail.com/

[6]
https://public-inbox.org/git/20180320175005.18759-1-ungureanupaulsebast
ian@gmail.com/T/#u

[7]
https://public-inbox.org/git/xmqqpo4ne8ud.fsf@gitster-ct.c.googlers.com
/

[8]
https://public-inbox.org/git/xmqqefkm6s06.fsf@gitster-ct.c.googlers.com
/

[9]
https://public-inbox.org/git/20160118215433.GB24136@sigill.intra.peff.n
et/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GSoC] Convert “git stash” to builtin proposal
  2018-03-20 20:09 [GSoC] Convert “git stash” to builtin proposal Paul-Sebastian Ungureanu
@ 2018-03-20 22:08 ` Christian Couder
  2018-03-22 23:15   ` Paul-Sebastian Ungureanu
  0 siblings, 1 reply; 11+ messages in thread
From: Christian Couder @ 2018-03-20 22:08 UTC (permalink / raw)
  To: Paul-Sebastian Ungureanu; +Cc: Git Mailing List, Johannes Schindelin

Hi,

On Tue, Mar 20, 2018 at 9:09 PM, Paul-Sebastian Ungureanu
<ungureanupaulsebastian@gmail.com> wrote:
>
> * Convert function: this step is basically makes up the goal of this
> project.

Could you explain a bit more how you would convert a function? Or
could you explain for example how you would convert "git stash list"
below?

> * Ensure that no regression occurred: considering that there are plenty
> of tests and that I have a good understanding of the function, this
> should be a trivial task.
>
> * Finally write more tests to ensure best code coverage.

Maybe, as Dscho suggested to another potential GSoC student, it would
be better to write more tests before converting the function.

> # Useful resources
> There has been a lot of progress made in this direction already and I
> believe it will serve of great help. However, from my understanding it
> is not yet mergeable and it requires changes.
>
> https://public-inbox.org/git/20170608005535.13080-1-joel@teichroeb.net/
> T/#m8849c7ce0ad8516cc206dd6910b79591bf9b3acd

Maybe you should Cc the author of this patch series.

Also please notice that the patch series started with adding some tests.

> I am expecting to submit a patch for every command that is converted.
> This way, I have well set milestones and results will be seen as soon
> as possible. Moreover, seeing my contributions getting merged will be a
> huge confidence boost to myself.

> Each “convert” phase of the project below is not only about converting
> from Shell to C, but also ensuring that everything is documented, there
> are enough tests and there are no regressions.
>
> 14th May - 20th May             - Convert "git stash list"

For example could you explain a bit more which functions/commands you
would create in which file and how you would call them to convert `git
stash list`

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GSoC] Convert “git stash” to builtin proposal
  2018-03-20 22:08 ` Christian Couder
@ 2018-03-22 23:15   ` Paul-Sebastian Ungureanu
  2018-03-23 17:06     ` Johannes Schindelin
  2018-03-23 17:11     ` Christian Couder
  0 siblings, 2 replies; 11+ messages in thread
From: Paul-Sebastian Ungureanu @ 2018-03-22 23:15 UTC (permalink / raw)
  To: Christian Couder; +Cc: Git Mailing List, Johannes Schindelin

Hello,

On Tue, 2018-03-20 at 23:08 +0100, Christian Couder wrote:
> Hi,
> 
> On Tue, Mar 20, 2018 at 9:09 PM, Paul-Sebastian Ungureanu
> <ungureanupaulsebastian@gmail.com> wrote:
> > 
> > * Convert function: this step is basically makes up the goal of
> > this
> > project.
> 
> Could you explain a bit more how you would convert a function? Or
> could you explain for example how you would convert "git stash list"
> below?

In order to convert a command, all the functions which are used by the
command must be converted first. The conversion will start with the
bottom-level functions, which do not have any dependencies.

For example, to convert "git stash list", the parser will call
“list_stash”, which will call “have_stash”. The conversion of these
functions will be made in reverse order they were mentioned (have_stash
first and then list_stash).

It is very important to know the Git source well in order to avoid
reimplementing functionality. In this case “have_stash()” is somehow
already implemented as “get_oid(ref_stash, &obj)”. 

> > I am expecting to submit a patch for every command that is
> > converted.
> > This way, I have well set milestones and results will be seen as
> > soon
> > as possible. Moreover, seeing my contributions getting merged will
> > be a
> > huge confidence boost to myself.
> > Each “convert” phase of the project below is not only about
> > converting
> > from Shell to C, but also ensuring that everything is documented,
> > there
> > are enough tests and there are no regressions.
> > 
> > 14th May - 20th May             - Convert "git stash list"
> 
> For example could you explain a bit more which functions/commands you
> would create in which file and how you would call them to convert
> `git
> stash list`

The new C code will be found in stash-helper.c and will be used by git-
stash.sh until the full conversion is complete. As soon as the entire
conversion is done, stash-helper.c will be promoted to stash.c. Any
functionality that will be implemented, but is not strictly related to
git stash will reside in the appropriate files (for example if I had to
implement similar to get_oid, which is not related to git stash, but to
Git, then I would not implement it in stash-helper.c; anyway, I do not
believe I will encounter this situation that often).

In the updated version of the proposal [1], I included the ideas stated
before and more information about the procces of benchmarking. In
addition, to test my capabilities and to be sure that I am able to work
on a project of this kind, I tried to convert “git stash list” into a
built-in (the code can be found in proposal). 

[1] https://docs.google.com/document/d/1TtdD7zgUsEOszHG5HX1at4zHMDsPmSB
YWqydOPTTpV8/edit

Convert “git stash” to builtin

## Name and Contact Information
------------------------------------------
Hello! My name is Paul-Sebastian Ungureanu. I am currently a first year
Computer Science & Engineering student at Politehnica University of
Bucharest, Romania.

My email address is ungureanupaulsebastian@gmail.com and my phone
number is [CENSORED]. You can also find me on #git IRC channel as
ungps.

FLOSS manual recommends students to include in their proposal their
postal address and mention a relative as a emergency contact. My
permanent address is [CENSORED]. In case of an emergency, you may
contact my brother, [CENSORED] by email at [CENSORED] or by phone at
[CENSORED].

## Synopsis
------------------------------------------
Currently, many components of Git are still in the form of Shell or
Perl scripts. This choice makes sense especially when considering how
faster new features can be implemented in Shell and Perl scripts,
rather than writing them in C. However, in production, where Git runs
daily on millions of computers with distinct configurations (i.e.
different operating systems) some problems appear (i.e. POSIX-to-
Windows path conversion issues).

The idea of this project is to take “git-stash.sh” and reimplement it
in C. Apart from fixing compatibility issues and increasing the
performance by going one step closer to native code, I believe this is
an excellent excuse to fix long-standing bugs or implement new minor
features.

## Benefits to community
------------------------------------------
The essential benefit brought by rewriting Git commands is the
increased compatibility with a large number computers with distinct
configuration. I believe that this project can have a positive impact
on a large mass of developers around the world. By rewriting the code
behind some popular commands and making them “built-in”, developers
will have a better overall experience when using Git and get to focus
on what really matters to them.

As a side effect, there will be a number of other improvements:
increased performance, ability to rethink the design of some code that
suffered from patching along the time, have the chance to create new
features and fix old bugs.

In theory, switching from Bash or Shell scripts, Git will be one step
closer to native code which should have a positive impact on the
performance. Being able to start from a clean slate is a great
opportunity to rethink old designs that may have been patched a lot
during their lifetime. This way, with the help of my mentors, I can
think of new ways to try and remove some limitations of the current
system (if there are any).

Moreover, I believe that the community will benefit greatly from new
features and bug fixes that I could help with. Even though this is not
really one of the main goals of this project, I believe that it would
be easier to fix bugs or implement new features while rewriting the
code. However, I will have to discuss with my mentors and carefully
review issues as I would not want to divagate from the purpose of the
project.

As a last point, I believe it is good to have a more homogenous
codebase, where the majority of the code would be written in C. This
could increase the number of contributions to the project as there are
maybe more programmers who are familiar with C, and not so much with
Perl or shell scripting.

## Deliverables
------------------------------------------
Deliverable of this project is “git stash” completely rewritten in
portable C code. Along with the new code, there will be some additions
and changes brought to tests to cover any new behaviour.

## Related work
------------------------------------------
Looking over the list of the other proposed projects, I believe that
“Convert interactive rebase to C” and “git bisect improvements” are the
most alike with this project and may be stretch goal of this project.

Moreover, there is a chance that other scripts could benefit from this
project if it were to be taken as an example for future conversions.

## Biographical information
------------------------------------------
I am a freshman at Politehnica University of Bucharest, which is
considered to be one of the most prestigious universities in the
Eastern Europe. I consider myself an ambitious software engineer that
enjoys competition. This has been proven by my participation to
programming competitions and extracurricular projects. As much as I
like competitions, I also love working with and meeting people that
share my interests for programming and technology.

Even though, in the last two years I found myself to be more interested
in Android software development rather than competitive programming, I
still take part in most of the competitions, such as contests organized
by Google HashCode, Google KickStart and ACM.

I have a good grasp of programming languages such as C and C++ and I
consider both of them to be my favorites. In the past, I intensively
used shell scripting and Git for some of my personal projects.

One of the facts that motivates me the most is that I get the chance to
improve my abilities by getting the code reviewed by professionals that
have been in the industry for a long period of time. This way, I get
first-hand experience regarding software management and increase my
programming skills in a professional environment.

Ultimately, I believe that being part of a community such as Git would
be a great confidence boost for me knowing that I gave something back
to an excellent piece of software that I am using on a daily basis.

## Project Details
------------------------------------------
# Plan for converting each command
* Understand the command well: read documentation, discover any
features I were not aware of, find out how it works and why it works
that way.

* Find out all edge and corner cases: these should be quite obvious as
soon as I have a good understanding of how the command works.

* Find out the history of the command and what future improvements can
be made: history of the command will help me know what issues affected
the command, how those issues were fixed and will help me avoid them in
the future. As I mentioned before, I believe that reimplementing those
commands is a great excuse to fix bugs and if there are any, I need to
know about them.

* Write tests to cover all possible cases. My process of development is
going to be similar to Test-Driven Development where a good amount of
tests is required.

* Convert function: this step basically makes up the goal of this
project. In order to convert a command, all the functions which are
used by the command must be converted first. The conversion will start
with the bottom-level functions, which do not have any dependencies.
The new C code will be found in stash-helper.c and will be used by git-
stash.sh until the full conversion is complete. As soon as the entire
conversion is done, stash-helper.c will be promoted to stash.c. Any
functionality that will be implemented, but is not strictly related to
git stash will reside in the appropriate files (for example if I had to
implement similar to get_oid, which is not related to git stash, but to
Git, then I would not implement it in stash-helper.c; anyway, I do not
believe I will encounter this situation that often).

* For example, to convert "git stash list", the parser will call
“list_stash”, which will call “have_stash”. The conversion of these
functions will be made in reverse order they were mentioned (have_stash
first and then list_stash).

* It is very important to know the Git source well in order to avoid
reimplementing functionality. In this case “have_stash()” is somehow
already implemented as “get_oid(ref_stash, &obj)”.

* Ensure that no regression occurred: considering that there are plenty
of tests and that I have a good understanding of the function, this
should be a trivial task.

* Write more tests to ensure the best code coverage. This step will be
almost non existent due to step 4.

* In the end, an analysis based on performance can be made.
Benchmarking the script will be done by recording the running time
given a large set of diversified tests that cover all the
functionalities, before and after conversion. The new commands should
run faster just because they were written in C, but I expect to see
even more improvements.

# Potential difficulties during conversion
------------------------------------------
* The rewritten code may introduce new bugs that may be delaying the
project. I believe there is a small chance this will happen as long as
I spend some time first to fully understand how it works and why it
works that way. Considering the amount of documentation already
available and the discussions from the mailing list, this should be a
trivial task. Moreover, I am already familiar with “git stash”.

* There may be old bugs, from the Shell scripts that would cause issues
while porting because they would probably require a discussion about
what implications the bug has and what is the best way to fix it.

* Most of the times, Shell scripts tend to be a lot more smaller and
hide a lot of complexity. Moreover, new data structures or utility
functions may be needed, some which exist and need changes or do not
exist and need to be implemented from scratch.

# Wrapping-up the project
------------------------------------------
In the end, the old Shell script will serve only as a called to the
stash-helper I would be implementing. The final step is to remove the
Shell script and promote the stash-helper to stash builtin.

## Example of conversion (for “git stash list”)
------------------------------------------
To test my capabilities and to be sure that I am able to work on a
project of this kind, I tried to convert “git stash list” into a built-
in. The result can be found below in text-only version or at the Github
link.

https://github.com/ungps/git/commit/3ffdece881525ee021a0f057a797e0717e1
01094

From 3ffdece881525ee021a0f057a797e0717e101094 Mon Sep 17 00:00:00 2001
From: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
Date: Wed, 21 Mar 2018 20:06:23 +0200
Subject: [PATCH] git-stash: convert git stash list from Shell script to
C
 builtin

Currently, because git stash is not fully converted to C, I
introduced a new helper that will hold the converted commands.
---
 Makefile                |  1 +
 builtin.h               |  1 +
 builtin/stash--helper.c | 52
+++++++++++++++++++++++++++++++++++++++++++++++++
 git-stash.sh            |  7 +------
 git.c                   |  1 +
 5 files changed, 56 insertions(+), 6 deletions(-)
 create mode 100644 builtin/stash--helper.c

diff --git a/Makefile b/Makefile
index a1d8775adb4b3..8ca361c57a8eb 100644
--- a/Makefile
+++ b/Makefile
@@ -1020,6 +1020,7 @@ BUILTIN_OBJS += builtin/send-pack.o
 BUILTIN_OBJS += builtin/shortlog.o
 BUILTIN_OBJS += builtin/show-branch.o
 BUILTIN_OBJS += builtin/show-ref.o
+BUILTIN_OBJS += builtin/stash--helper.o
 BUILTIN_OBJS += builtin/stripspace.o
 BUILTIN_OBJS += builtin/submodule--helper.o
 BUILTIN_OBJS += builtin/symbolic-ref.o
diff --git a/builtin.h b/builtin.h
index 42378f3aa471e..2ddb4bd5c76fa 100644
--- a/builtin.h
+++ b/builtin.h
@@ -220,6 +220,7 @@ extern int cmd_show(int argc, const char **argv,
const char *prefix);
 extern int cmd_show_branch(int argc, const char **argv, const char
*prefix);
 extern int cmd_status(int argc, const char **argv, const char
*prefix);
 extern int cmd_stripspace(int argc, const char **argv, const char
*prefix);
+extern int cmd_stash__helper(int argc, const char **argv, const char
*prefix);
 extern int cmd_submodule__helper(int argc, const char **argv, const
char *prefix);
 extern int cmd_symbolic_ref(int argc, const char **argv, const char
*prefix);
 extern int cmd_tag(int argc, const char **argv, const char *prefix);
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
new file mode 100644
index 0000000000000..61fd5390d7d61
--- /dev/null
+++ b/builtin/stash--helper.c
@@ -0,0 +1,52 @@
+#include "builtin.h"
+#include "cache.h"
+#include "parse-options.h"
+#include "argv-array.h"
+
+enum {
+	LIST_STASH = 1
+};
+
+static const char * ref_stash = "refs/stash";
+
+static const char * const git_stash__helper_usage[] = {
+	N_("git stash--helper --list [<options>]"),
+	NULL
+};
+
+static int list_stash(int argc, const char **argv, const char *prefix)
+{
+	struct object_id obj;
+	struct argv_array args = ARGV_ARRAY_INIT;
+
+	if (get_oid(ref_stash, &obj))
+		return 0;
+
+	argv_array_pushl(&args, "log", "--format=%gd: %gs", "-g", "
--first-parent", "-m", NULL);
+	argv_array_pushv(&args, argv);
+	argv_array_push(&args, ref_stash);
+	return !!cmd_log(args.argc, args.argv, prefix);
+}
+
+int cmd_stash__helper(int argc, const char **argv, const char *prefix)
+{
+	int cmdmode = 0;
+
+	struct option options[] = {
+		OPT_CMDMODE(0, "list", &cmdmode,
+			 N_("list stash entries"), LIST_STASH),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options,
+			     git_stash__helper_usage,
PARSE_OPT_KEEP_UNKNOWN);
+
+	if (!cmdmode)
+		usage_with_options(git_stash__helper_usage, options);
+
+	switch (cmdmode) {
+		case LIST_STASH:
+			return list_stash(argc, argv, prefix);
+	}
+	return 0;
+}
diff --git a/git-stash.sh b/git-stash.sh
index fc8f8ae6401dd..a5b9f5fb60bba 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -380,11 +380,6 @@ have_stash () {
 	git rev-parse --verify --quiet $ref_stash >/dev/null
 }

-list_stash () {
-	have_stash || return 0
-	git log --format="%gd: %gs" -g --first-parent -m "$@"
$ref_stash --
-}
-
 show_stash () {
 	ALLOW_UNKNOWN_FLAGS=t
 	assert_stash_like "$@"
@@ -695,7 +690,7 @@ test -n "$seen_non_option" || set "push" "$@"
 case "$1" in
 list)
 	shift
-	list_stash "$@"
+	git stash--helper --list "$@"
 	;;
 show)
 	shift
diff --git a/git.c b/git.c
index 96cd734f12821..6fd2ccd9a81bc 100644
--- a/git.c
+++ b/git.c
@@ -466,6 +466,7 @@ static struct cmd_struct commands[] = {
 	{ "show-branch", cmd_show_branch, RUN_SETUP },
 	{ "show-ref", cmd_show_ref, RUN_SETUP },
 	{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
+	{ "stash--helper", cmd_stash__helper, RUN_SETUP },
 	{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
 	{ "stripspace", cmd_stripspace },
 	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP |
SUPPORT_SUPER_PREFIX},

Useful resources
There has been a lot of progress made in this direction already and I
believe it will serve of great help. However, from my understanding it
is not yet mergeable and it requires changes.

https://public-inbox.org/git/20170608005535.13080-1-joel@teichroeb.net/
T/#m8849c7ce0ad8516cc206dd6910b79591bf9b3acd

## Project Schedule
------------------------------------------
I can spend about 25 hours during the exam period (last half of May and
the first half of June) and about 40 hours a week as soon as I am done
with exams. I hope that by the start of the program (14th of May), I
will have completed a good part of the project and that will compensate
for the exam period.

After Google Summer of Code ends, I will do my best to keep
contributing to Git. Hopefully, all the work I proposed over the summer
will be done and I will be able to move towards other areas, maybe by
continuing converting other scripts to builtins or something else,
depending on what are the priorities of the Git team.

Google Summer of Code starts on 14th of May and lasts 13 weeks. I
propose to convert one command per week. I actually do not expect each
command to take exactly one week, some may be easier to convert and
other may be more challenging, but I believe that the average time of
conversion is around 1 week.

I am expecting to submit a patch for every command that is converted.
This way, I have well set milestones and results will be seen as soon
as possible. Moreover, seeing my contributions getting merged will be a
huge confidence boost to myself.

Each “convert” phase of the project below is not only about converting
from Shell to C, but also ensuring that everything is documented, there
are enough tests and there are no regressions.

14th May - 20th May     	- Convert "git stash list"
21st May - 27th May     	- Convert "git stash show"
28th May - 3rd June     	- Convert "git stash save"
4th June - 10th June    	- Convert "git stash push"
11th June - 17th June*  	- Convert "git stash apply"
18th June - 24th June   	- Convert "git stash clear"
25th June - 1st July    	- Convert "git stash create"
2nd July - 8th July     	- Convert "git stash store"
9th July - 15th July*  		- Convert "git stash drop"
16th July - 22nd July    	- Convert "git stash pop"
23rd July - 29th July      	- Convert "git stash branch"
30th July - 5th August 		- Remove old Shell script.
6th August - 12th August* 	- Wrap-up the project by writing more
tests and documentation.
12th August - forever 		- Pick up another project.

* 1st, 2nd and 3rd mentor and student evaluation

## Git contributions
------------------------------------------
My first contribution to Git was to help making “git tag --contains
<id>” les chatty if <id> is invalid. Looking over the list of available
microprojects, there were a couple of microprojects which got my
attention, but I picked this up because it seemed to be a long-standing 
bug (I noticed it was approached by students in 2016, 2017 and now in
2018). Thanks to the code reviews from the mailing list, after a few
iterations I succeeded in coming up with a patch that not only fixed
the mentioned problem, but also a few others that were having the same
cause.

It got merged in the proposed updates branch.

I worked on only one project because I tried to achieve the best
quality I could. Even after I submitted one patch that was good enough
[8], I decided to try another approach that worked better in the end
[9].

First of all, I weighed the ideas mentioned at [10] and considered the
second one to be the best. After a few iterations [1], [2], [3], [4],
[5], [6] and after Junio’s review I decided to try another approach and
submitted another patch [7].

I changed the approach, because this issue was affecting not only “git
tag” command, but also “git for-each-ref”, “git branch” and many more.
The new submission was based on Junio’s ideea and fixed the cause at a
lower level than the previous patch.

In order to make sure that the new code was 100% correct and
familiarize myself with the testing system, I also write a new set of
tests.

[1]
https://public-inbox.org/git/20180219212130.4217-1-ungureanupaulsebasti
an@gmail.com/

[2]
https://public-inbox.org/git/20180223162557.31477-1-ungureanupaulsebast
ian@gmail.com/

[3]
https://public-inbox.org/git/20180303210938.32474-1-ungureanupaulsebast
ian@gmail.com/

[4]
https://public-inbox.org/git/20180306193116.23876-1-ungureanupaulsebast
ian@gmail.com/

[5]
https://public-inbox.org/git/20180319185436.14309-1-ungureanupaulsebast
ian@gmail.com/

[6]
https://public-inbox.org/git/20180320175005.18759-1-ungureanupaulsebast
ian@gmail.com/

[7]
https://public-inbox.org/git/20180322184351.2392-1-ungureanupaulsebasti
an@gmail.com/

[8]
https://public-inbox.org/git/xmqqpo4ne8ud.fsf@gitster-ct.c.googlers.com
/

[9]
https://public-inbox.org/git/xmqqefkm6s06.fsf@gitster-ct.c.googlers.com
/

[10]
https://public-inbox.org/git/20160118215433.GB24136@sigill.intra.peff.n
et/

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [GSoC] Convert “git stash” to builtin proposal
  2018-03-22 23:15   ` Paul-Sebastian Ungureanu
@ 2018-03-23 17:06     ` Johannes Schindelin
  2018-03-25 14:32       ` Paul-Sebastian Ungureanu
  2018-03-23 17:11     ` Christian Couder
  1 sibling, 1 reply; 11+ messages in thread
From: Johannes Schindelin @ 2018-03-23 17:06 UTC (permalink / raw)
  To: Paul-Sebastian Ungureanu; +Cc: Christian Couder, Git Mailing List

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

Hi Paul-Sebastian,

On Fri, 23 Mar 2018, Paul-Sebastian Ungureanu wrote:

> On Tue, 2018-03-20 at 23:08 +0100, Christian Couder wrote:
> > Hi,
> > 
> > On Tue, Mar 20, 2018 at 9:09 PM, Paul-Sebastian Ungureanu
> > <ungureanupaulsebastian@gmail.com> wrote:
> > > 
> > > * Convert function: this step is basically makes up the goal of
> > > this
> > > project.
> > 
> > Could you explain a bit more how you would convert a function? Or
> > could you explain for example how you would convert "git stash list"
> > below?
> 
> In order to convert a command, all the functions which are used by the
> command must be converted first. The conversion will start with the
> bottom-level functions, which do not have any dependencies.
> 
> For example, to convert "git stash list", the parser will call
> “list_stash”, which will call “have_stash”. The conversion of these
> functions will be made in reverse order they were mentioned (have_stash
> first and then list_stash).
> 
> It is very important to know the Git source well in order to avoid
> reimplementing functionality. In this case “have_stash()” is somehow
> already implemented as “get_oid(ref_stash, &obj)”. 

Very good

> [... proposal ...]

This is a pretty good proposal. The initial draft at converting `stash
list` is a good start (it will need to be converted to avoid spawning an
extra process, but that is something we can do incrementally, together).

Ciao,
Johannes

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GSoC] Convert “git stash” to builtin proposal
  2018-03-22 23:15   ` Paul-Sebastian Ungureanu
  2018-03-23 17:06     ` Johannes Schindelin
@ 2018-03-23 17:11     ` Christian Couder
  2018-03-24 19:31       ` Paul-Sebastian Ungureanu
  1 sibling, 1 reply; 11+ messages in thread
From: Christian Couder @ 2018-03-23 17:11 UTC (permalink / raw)
  To: Paul-Sebastian Ungureanu; +Cc: Git Mailing List, Johannes Schindelin

On Fri, Mar 23, 2018 at 12:15 AM, Paul-Sebastian Ungureanu
<ungureanupaulsebastian@gmail.com> wrote:
>
> On Tue, 2018-03-20 at 23:08 +0100, Christian Couder wrote:
>>
>> On Tue, Mar 20, 2018 at 9:09 PM, Paul-Sebastian Ungureanu
>> <ungureanupaulsebastian@gmail.com> wrote:
>> >
>> > * Convert function: this step is basically makes up the goal of
>> > this
>> > project.
>>
>> Could you explain a bit more how you would convert a function? Or
>> could you explain for example how you would convert "git stash list"
>> below?
>
> In order to convert a command, all the functions which are used by the
> command must be converted first. The conversion will start with the
> bottom-level functions, which do not have any dependencies.
>
> For example, to convert "git stash list", the parser will call
> “list_stash”, which will call “have_stash”. The conversion of these
> functions will be made in reverse order they were mentioned (have_stash
> first and then list_stash).

Ok.

> It is very important to know the Git source well in order to avoid
> reimplementing functionality. In this case “have_stash()” is somehow
> already implemented as “get_oid(ref_stash, &obj)”.
>
>> > I am expecting to submit a patch for every command that is
>> > converted.
>> > This way, I have well set milestones and results will be seen as
>> > soon
>> > as possible. Moreover, seeing my contributions getting merged will
>> > be a
>> > huge confidence boost to myself.
>> > Each “convert” phase of the project below is not only about
>> > converting
>> > from Shell to C, but also ensuring that everything is documented,
>> > there
>> > are enough tests and there are no regressions.
>> >
>> > 14th May - 20th May             - Convert "git stash list"
>>
>> For example could you explain a bit more which functions/commands you
>> would create in which file and how you would call them to convert
>> `git
>> stash list`
>
> The new C code will be found in stash-helper.c and will be used by git-
> stash.sh until the full conversion is complete. As soon as the entire
> conversion is done, stash-helper.c will be promoted to stash.c. Any
> functionality that will be implemented, but is not strictly related to
> git stash will reside in the appropriate files (for example if I had to
> implement similar to get_oid, which is not related to git stash, but to
> Git, then I would not implement it in stash-helper.c; anyway, I do not
> believe I will encounter this situation that often).

Ok.

> In the updated version of the proposal [1], I included the ideas stated
> before and more information about the procces of benchmarking. In
> addition, to test my capabilities and to be sure that I am able to work
> on a project of this kind, I tried to convert “git stash list” into a
> built-in (the code can be found in proposal).
>
> [1] https://docs.google.com/document/d/1TtdD7zgUsEOszHG5HX1at4zHMDsPmSB
> YWqydOPTTpV8/edit

I guess that below you inlined the updated version of your proposal. Nice.

> Convert “git stash” to builtin

[...]

> * Ensure that no regression occurred: considering that there are plenty
> of tests and that I have a good understanding of the function, this
> should be a trivial task.

There are a lot of things that the test suite doesn't test.

> * Write more tests to ensure the best code coverage. This step will be
> almost non existent due to step 4.

Maybe you should use a numbered list if you want to refer to the items
by their number.

> * In the end, an analysis based on performance can be made.
> Benchmarking the script will be done by recording the running time
> given a large set of diversified tests that cover all the
> functionalities, before and after conversion. The new commands should
> run faster just because they were written in C, but I expect to see
> even more improvements.

If you want to do benchmarks, you might want to add performance tests
into t/perf.

[...]

> ## Example of conversion (for “git stash list”)
> ------------------------------------------
> To test my capabilities and to be sure that I am able to work on a
> project of this kind, I tried to convert “git stash list” into a built-
> in. The result can be found below in text-only version or at the Github
> link.

I think it would be better if it was sent as a patch (maybe an RFC
patch) to the mailing list and add the link to the patch email in the
maling list archive to your proposal.

[...]

> Useful resources
> There has been a lot of progress made in this direction already and I
> believe it will serve of great help. However, from my understanding it
> is not yet mergeable and it requires changes.
>
> https://public-inbox.org/git/20170608005535.13080-1-joel@teichroeb.net/
> T/#m8849c7ce0ad8516cc206dd6910b79591bf9b3acd

It could be useful if you described a bit more how you could (re)use
the work that has already been made.

In the rest of your proposal it looks like you are starting from
scratch, which is a bit strange if a lot of progress has already been
made.

> ## Project Schedule
> ------------------------------------------
> I can spend about 25 hours during the exam period (last half of May and
> the first half of June) and about 40 hours a week as soon as I am done
> with exams. I hope that by the start of the program (14th of May), I
> will have completed a good part of the project and that will compensate
> for the exam period.
>
> After Google Summer of Code ends, I will do my best to keep
> contributing to Git. Hopefully, all the work I proposed over the summer
> will be done and I will be able to move towards other areas, maybe by
> continuing converting other scripts to builtins or something else,
> depending on what are the priorities of the Git team.
>
> Google Summer of Code starts on 14th of May and lasts 13 weeks. I
> propose to convert one command per week. I actually do not expect each
> command to take exactly one week, some may be easier to convert and
> other may be more challenging, but I believe that the average time of
> conversion is around 1 week.
>
> I am expecting to submit a patch for every command that is converted.
> This way, I have well set milestones and results will be seen as soon
> as possible. Moreover, seeing my contributions getting merged will be a
> huge confidence boost to myself.
>
> Each “convert” phase of the project below is not only about converting
> from Shell to C, but also ensuring that everything is documented, there
> are enough tests and there are no regressions.
>
> 14th May - 20th May             - Convert "git stash list"
> 21st May - 27th May             - Convert "git stash show"
> 28th May - 3rd June             - Convert "git stash save"
> 4th June - 10th June            - Convert "git stash push"
> 11th June - 17th June*          - Convert "git stash apply"
> 18th June - 24th June           - Convert "git stash clear"
> 25th June - 1st July            - Convert "git stash create"
> 2nd July - 8th July             - Convert "git stash store"
> 9th July - 15th July*           - Convert "git stash drop"
> 16th July - 22nd July           - Convert "git stash pop"
> 23rd July - 29th July           - Convert "git stash branch"
> 30th July - 5th August          - Remove old Shell script.
> 6th August - 12th August*       - Wrap-up the project by writing more
> tests and documentation.
> 12th August - forever           - Pick up another project.
>
> * 1st, 2nd and 3rd mentor and student evaluation

It is probably better especially for reviewers and more common to work
in small batches, for example to send a patch series converting a few
related commands, then to start working on the next small batch of
commands in another patch series while improving the first patch
series according to reviews.

Also we ask GSoC students to communicate publicly every week about
their project for example by writing a blg post or sending a report to
the mailing list.

> ## Git contributions
> ------------------------------------------
> My first contribution to Git was to help making “git tag --contains
> <id>” les chatty if <id> is invalid. Looking over the list of available
> microprojects, there were a couple of microprojects which got my
> attention, but I picked this up because it seemed to be a long-standing
> bug (I noticed it was approached by students in 2016, 2017 and now in
> 2018). Thanks to the code reviews from the mailing list, after a few
> iterations I succeeded in coming up with a patch that not only fixed
> the mentioned problem, but also a few others that were having the same
> cause.
>
> It got merged in the proposed updates branch.

What is its status in Junio's "What's cooking in Git" emails?

> I worked on only one project because I tried to achieve the best
> quality I could. Even after I submitted one patch that was good enough
> [8], I decided to try another approach that worked better in the end
> [9].
>
> First of all, I weighed the ideas mentioned at [10] and considered the
> second one to be the best. After a few iterations [1], [2], [3], [4],
> [5], [6] and after Junio’s review I decided to try another approach and
> submitted another patch [7].
>
> I changed the approach, because this issue was affecting not only “git
> tag” command, but also “git for-each-ref”, “git branch” and many more.
> The new submission was based on Junio’s ideea and fixed the cause at a
> lower level than the previous patch.
>
> In order to make sure that the new code was 100% correct and
> familiarize myself with the testing system, I also write a new set of
> tests.

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GSoC] Convert “git stash” to builtin proposal
  2018-03-23 17:11     ` Christian Couder
@ 2018-03-24 19:31       ` Paul-Sebastian Ungureanu
  2018-03-25  9:46         ` Christian Couder
  0 siblings, 1 reply; 11+ messages in thread
From: Paul-Sebastian Ungureanu @ 2018-03-24 19:31 UTC (permalink / raw)
  To: Christian Couder; +Cc: Git Mailing List, Johannes Schindelin

On 23.03.2018 19:11, Christian Couder wrote:

>> * Ensure that no regression occurred: considering that there are plenty
>> of tests and that I have a good understanding of the function, this
>> should be a trivial task.
> 
> There are a lot of things that the test suite doesn't test.

Hopefully, by first adding new tests, any eventual bug will be spotted.

>> * In the end, an analysis based on performance can be made.
>> Benchmarking the script will be done by recording the running time
>> given a large set of diversified tests that cover all the
>> functionalities, before and after conversion. The new commands should
>> run faster just because they were written in C, but I expect to see
>> even more improvements.
> 
> If you want to do benchmarks, you might want to add performance tests
> into t/perf.

That is great. I will surely make use of the existent testing framework 
to do benchmarks.

>> ## Example of conversion (for “git stash list”)
>> ------------------------------------------
>> To test my capabilities and to be sure that I am able to work on a
>> project of this kind, I tried to convert “git stash list” into a built-
>> in. The result can be found below in text-only version or at the Github
>> link.
> 
> I think it would be better if it was sent as a patch (maybe an RFC
> patch) to the mailing list and add the link to the patch email in the
> maling list archive to your proposal.

I sent it as a [RFC] patch to the mailing list and included it in the 
proposal.

<https://public-inbox.org/git/20180324182313.13705-1-ungureanupaulsebastian@gmail.com>

> It could be useful if you described a bit more how you could (re)use
> the work that has already been made.
> 
> In the rest of your proposal it looks like you are starting from
> scratch, which is a bit strange if a lot of progress has already been
> made.

The patches about converting ‘git stash’ are a great starting point and 
I will definitely use them. Each time I start converting a new command I 
will first take a look at what other patches there are, evaluate them 
and if I consider the patch to be good enough I will continue my work on 
top of that patch. Otherwise, I will start from scratch and that patch 
will only serve for comparison.

One other resource that is of great importance is git itself. I can 
learn how a builtin is structured and how it is built by considering 
examples the other Git commands. Having a similar coding standard used, 
the codebase will be homogeneous and hopefully easier to maintain.

Another important resource are commands that are Google Summer of Code 
projects from previous years (2016 and 2017) which had as purpose to 
convert “git bisect” and “git submodule”. I can always take a look at 
the patches they submitted and read their code reviews to avoid making 
same mistakes they did.

> It is probably better especially for reviewers and more common to work
> in small batches, for example to send a patch series converting a few
> related commands, then to start working on the next small batch of
> commands in another patch series while improving the first patch
> series according to reviews.
> 
> Also we ask GSoC students to communicate publicly every week about
> their project for example by writing a blg post or sending a report to
> the mailing list.

Noted.

>> ## Git contributions
>> ------------------------------------------
>> My first contribution to Git was to help making “git tag --contains
>> <id>” les chatty if <id> is invalid. Looking over the list of available
>> microprojects, there were a couple of microprojects which got my
>> attention, but I picked this up because it seemed to be a long-standing
>> bug (I noticed it was approached by students in 2016, 2017 and now in
>> 2018). Thanks to the code reviews from the mailing list, after a few
>> iterations I succeeded in coming up with a patch that not only fixed
>> the mentioned problem, but also a few others that were having the same
>> cause.
>>
>> It got merged in the proposed updates branch.
> 
> What is its status in Junio's "What's cooking in Git" emails?

It is now in the ‘next’ branch of the Git repository.

I updated the proposal, in which I included these ideas and some 
additional examples. Thank you a lot!

Best regards,
Paul Ungureanu

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GSoC] Convert “git stash” to builtin proposal
  2018-03-24 19:31       ` Paul-Sebastian Ungureanu
@ 2018-03-25  9:46         ` Christian Couder
  2018-03-25 14:38           ` Paul-Sebastian Ungureanu
  0 siblings, 1 reply; 11+ messages in thread
From: Christian Couder @ 2018-03-25  9:46 UTC (permalink / raw)
  To: Paul-Sebastian Ungureanu; +Cc: Git Mailing List, Johannes Schindelin

On Sat, Mar 24, 2018 at 8:31 PM, Paul-Sebastian Ungureanu
<ungureanupaulsebastian@gmail.com> wrote:
> On 23.03.2018 19:11, Christian Couder wrote:
>
>>> * Ensure that no regression occurred: considering that there are plenty
>>> of tests and that I have a good understanding of the function, this
>>> should be a trivial task.
>>
>> There are a lot of things that the test suite doesn't test.
>
> Hopefully, by first adding new tests, any eventual bug will be spotted.

I was thinking about things like memory leaks that tests cannot easily spot.

>>> * In the end, an analysis based on performance can be made.
>>> Benchmarking the script will be done by recording the running time
>>> given a large set of diversified tests that cover all the
>>> functionalities, before and after conversion. The new commands should
>>> run faster just because they were written in C, but I expect to see
>>> even more improvements.
>>
>> If you want to do benchmarks, you might want to add performance tests
>> into t/perf.
>
> That is great. I will surely make use of the existent testing framework to
> do benchmarks.

Good.

>>> ## Example of conversion (for “git stash list”)
>>> ------------------------------------------
>>> To test my capabilities and to be sure that I am able to work on a
>>> project of this kind, I tried to convert “git stash list” into a built-
>>> in. The result can be found below in text-only version or at the Github
>>> link.
>>
>> I think it would be better if it was sent as a patch (maybe an RFC
>> patch) to the mailing list and add the link to the patch email in the
>> maling list archive to your proposal.
>
> I sent it as a [RFC] patch to the mailing list and included it in the
> proposal.
>
> <https://public-inbox.org/git/20180324182313.13705-1-ungureanupaulsebastian@gmail.com>

Nice.

>> It could be useful if you described a bit more how you could (re)use
>> the work that has already been made.
>>
>> In the rest of your proposal it looks like you are starting from
>> scratch, which is a bit strange if a lot of progress has already been
>> made.
>
> The patches about converting ‘git stash’ are a great starting point and I
> will definitely use them. Each time I start converting a new command I will
> first take a look at what other patches there are, evaluate them and if I
> consider the patch to be good enough I will continue my work on top of that
> patch. Otherwise, I will start from scratch and that patch will only serve
> for comparison.
>
> One other resource that is of great importance is git itself. I can learn
> how a builtin is structured and how it is built by considering examples the
> other Git commands. Having a similar coding standard used, the codebase will
> be homogeneous and hopefully easier to maintain.
>
> Another important resource are commands that are Google Summer of Code
> projects from previous years (2016 and 2017) which had as purpose to convert
> “git bisect” and “git submodule”. I can always take a look at the patches
> they submitted and read their code reviews to avoid making same mistakes
> they did.

Nice.

>> It is probably better especially for reviewers and more common to work
>> in small batches, for example to send a patch series converting a few
>> related commands, then to start working on the next small batch of
>> commands in another patch series while improving the first patch
>> series according to reviews.
>>
>> Also we ask GSoC students to communicate publicly every week about
>> their project for example by writing a blg post or sending a report to
>> the mailing list.
>
> Noted.
>
>>> ## Git contributions
>>> ------------------------------------------
>>> My first contribution to Git was to help making “git tag --contains
>>> <id>” les chatty if <id> is invalid. Looking over the list of available
>>> microprojects, there were a couple of microprojects which got my
>>> attention, but I picked this up because it seemed to be a long-standing
>>> bug (I noticed it was approached by students in 2016, 2017 and now in
>>> 2018). Thanks to the code reviews from the mailing list, after a few
>>> iterations I succeeded in coming up with a patch that not only fixed
>>> the mentioned problem, but also a few others that were having the same
>>> cause.
>>>
>>> It got merged in the proposed updates branch.
>>
>> What is its status in Junio's "What's cooking in Git" emails?
>
> It is now in the ‘next’ branch of the Git repository.
>
> I updated the proposal, in which I included these ideas and some additional
> examples. Thank you a lot!

Ok. Feel free to resend another version of your proposal.

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GSoC] Convert “git stash” to builtin proposal
  2018-03-23 17:06     ` Johannes Schindelin
@ 2018-03-25 14:32       ` Paul-Sebastian Ungureanu
  0 siblings, 0 replies; 11+ messages in thread
From: Paul-Sebastian Ungureanu @ 2018-03-25 14:32 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Christian Couder, Git Mailing List

On 23.03.2018 19:06, Johannes Schindelin wrote:
>> [... proposal ...]
> 
> This is a pretty good proposal. The initial draft at converting `stash
> list` is a good start (it will need to be converted to avoid spawning an
> extra process, but that is something we can do incrementally, together).

Thank you for your kind words. It feels good to see other people 
appreciate my work. It is a strong incentive to keep going on. I made a 
few adjustments and I hope that the final version will be better.

Best regards,
Paul Ungureanu

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GSoC] Convert “git stash” to builtin proposal
  2018-03-25  9:46         ` Christian Couder
@ 2018-03-25 14:38           ` Paul-Sebastian Ungureanu
  2018-03-26 22:04             ` Johannes Schindelin
  2018-03-27  6:14             ` Christian Couder
  0 siblings, 2 replies; 11+ messages in thread
From: Paul-Sebastian Ungureanu @ 2018-03-25 14:38 UTC (permalink / raw)
  To: Christian Couder; +Cc: Git Mailing List, Johannes Schindelin

On 25.03.2018 12:46, Christian Couder wrote:
> On Sat, Mar 24, 2018 at 8:31 PM, Paul-Sebastian Ungureanu
> <ungureanupaulsebastian@gmail.com> wrote:
>> On 23.03.2018 19:11, Christian Couder wrote:
>>
>>>> * Ensure that no regression occurred: considering that there are plenty
>>>> of tests and that I have a good understanding of the function, this
>>>> should be a trivial task.
>>>
>>> There are a lot of things that the test suite doesn't test.
>>
>> Hopefully, by first adding new tests, any eventual bug will be spotted.
> 
> I was thinking about things like memory leaks that tests cannot easily spot.> 

I will do my best and follow best practices in order to avoid any memory 
leaks. However, to make sure that my code is really leak free, I will 
use Valgrind, which is already integrated with the testing framework.

> Ok. Feel free to resend another version of your proposal.
Sorry for not sending the whole proposal again. I decided to send only 
the changed parts because the proposal is quite big and I wanted to 
avoid sending the same thing over and over again.

One thing I did not mention in the previous reply was that I also added 
a new paragraph to "Benefits to community" about 'git stash' being slow 
on Windows for a lot of users. I consider this alone to be a very good 
justification for this project and doing this project will be very 
beneficial for the Windows users.

Best regards,
Paul Ungureanu

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GSoC] Convert “git stash” to builtin proposal
  2018-03-25 14:38           ` Paul-Sebastian Ungureanu
@ 2018-03-26 22:04             ` Johannes Schindelin
  2018-03-27  6:14             ` Christian Couder
  1 sibling, 0 replies; 11+ messages in thread
From: Johannes Schindelin @ 2018-03-26 22:04 UTC (permalink / raw)
  To: Paul-Sebastian Ungureanu; +Cc: Christian Couder, Git Mailing List

Hi Paul,

On Sun, 25 Mar 2018, Paul-Sebastian Ungureanu wrote:

> One thing I did not mention in the previous reply was that I also added
> a new paragraph to "Benefits to community" about 'git stash' being slow
> on Windows for a lot of users. I consider this alone to be a very good
> justification for this project and doing this project will be very
> beneficial for the Windows users.

Yesssss!

Thank you,
Johannes

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GSoC] Convert “git stash” to builtin proposal
  2018-03-25 14:38           ` Paul-Sebastian Ungureanu
  2018-03-26 22:04             ` Johannes Schindelin
@ 2018-03-27  6:14             ` Christian Couder
  1 sibling, 0 replies; 11+ messages in thread
From: Christian Couder @ 2018-03-27  6:14 UTC (permalink / raw)
  To: Paul-Sebastian Ungureanu; +Cc: Git Mailing List, Johannes Schindelin

On Sun, Mar 25, 2018 at 4:38 PM, Paul-Sebastian Ungureanu
<ungureanupaulsebastian@gmail.com> wrote:
> On 25.03.2018 12:46, Christian Couder wrote:
>>
>> On Sat, Mar 24, 2018 at 8:31 PM, Paul-Sebastian Ungureanu
>> <ungureanupaulsebastian@gmail.com> wrote:
>>>
>>> On 23.03.2018 19:11, Christian Couder wrote:
>>>
>>>>> * Ensure that no regression occurred: considering that there are plenty
>>>>> of tests and that I have a good understanding of the function, this
>>>>> should be a trivial task.
>>>>
>>>> There are a lot of things that the test suite doesn't test.
>>>
>>> Hopefully, by first adding new tests, any eventual bug will be spotted.
>>
>> I was thinking about things like memory leaks that tests cannot easily
>> spot.>
>
> I will do my best and follow best practices in order to avoid any memory
> leaks. However, to make sure that my code is really leak free, I will use
> Valgrind, which is already integrated with the testing framework.

Yeah, but it's still not so easy even with valgrind for a number of
reasons. For example it's difficult to test all the error paths, and
there are also some memory leaks that we accept when we know that they
happen just once and that we are going to exit soon anyway when we
should free the memory.

>> Ok. Feel free to resend another version of your proposal.
>
> Sorry for not sending the whole proposal again. I decided to send only the
> changed parts because the proposal is quite big and I wanted to avoid
> sending the same thing over and over again.

It's up to you, but but fewer people might review it.

> One thing I did not mention in the previous reply was that I also added a
> new paragraph to "Benefits to community" about 'git stash' being slow on
> Windows for a lot of users. I consider this alone to be a very good
> justification for this project and doing this project will be very
> beneficial for the Windows users.

Sure.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-03-27  6:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-20 20:09 [GSoC] Convert “git stash” to builtin proposal Paul-Sebastian Ungureanu
2018-03-20 22:08 ` Christian Couder
2018-03-22 23:15   ` Paul-Sebastian Ungureanu
2018-03-23 17:06     ` Johannes Schindelin
2018-03-25 14:32       ` Paul-Sebastian Ungureanu
2018-03-23 17:11     ` Christian Couder
2018-03-24 19:31       ` Paul-Sebastian Ungureanu
2018-03-25  9:46         ` Christian Couder
2018-03-25 14:38           ` Paul-Sebastian Ungureanu
2018-03-26 22:04             ` Johannes Schindelin
2018-03-27  6:14             ` Christian Couder

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).