{"thread":{"id":"48101","subject":"[GSoC] Convert “git stash” to builtin proposal","startedAt":"2018-03-20T20:09:32Z","lastAt":"2018-03-27T06:14:42Z","messageCount":11,"participants":["Paul-Sebastian Ungureanu","Christian Couder","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"342358","messageId":"1521576562.2188.10.camel@gmail.com","threadId":"48101","inReplyTo":null,"subject":"[GSoC] Convert “git stash” to builtin proposal","fromName":"Paul-Sebastian Ungureanu","fromEmail":"ungureanupaulsebastian@gmail.com","sentAt":"2018-03-20T20:09:22Z","receivedAt":"2018-03-20T20:09:32Z","isPatch":false,"sender":{"key":"ungureanupaulsebastian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/24317622?v=4"},"body":"Hello,\n\nI have completed the first draft of my proposal for the Google Summer\nof Code, which can be found at [1] or below for those who prefer the\ntext version.\n\nAny feedback is greatly appreciated. Thanks!\n\n[1]\nhttps://docs.google.com/document/d/1TtdD7zgUsEOszHG5HX1at4zHMDsPmSBYWqy\ndOPTTpV8/edit?usp=sharing\n\nBest regards,\nPaul-Sebastian Ungureanu\n\n# Convert “git stash” to builtin\n\n# Name and Contact Information\nHello! My name is Paul-Sebastian Ungureanu. I am currently a first year\nComputer Science & Engineering student at Politehnica University of\nBucharest, Romania.\n\nMy email address is ungureanupaulsebastian@gmail.com and my phone\nnumber is [CENSORED]. You can also find me on #git IRC channel as\nungps.\n\nFLOSS manual recommends students to include in their proposal their\npostal address and mention a relative as a emergency contact. My\npermanent address is [CENSORED]. In case of an emergency, you may\ncontact my brother, [CENSORED] by email at [CENSORED] or by phone at\n[CENSORED].\n\n# Synopsis\nCurrently, many components of Git are still in the form of Shell or\nPerl scripts. This choice makes sense especially when considering how\nfaster new features can be implemented in Shell and Perl scripts,\nrather than writing them in C. However, in production, where Git runs\ndaily on millions of computers with distinct configurations (i.e.\ndifferent operating systems) some problems appear (i.e. POSIX-to-\nWindows path conversion issues).\n\nThe idea of this project is to take “git-stash.sh” and reimplement it\nin C. Apart from fixing compatibility issues and increasing the\nperformance by going one step closer to native code, I believe this is\nan excellent excuse to fix long-standing bugs or implement new minor\nfeatures.\n\n# Benefits to community\nThe essential benefit brought by rewriting Git commands is the\nincreased compatibility with a large number computers with distinct\nconfiguration. I believe that this project can have a positive impact\non a large mass of developers around the world. By rewriting the code\nbehind some popular commands and making them “built-in”, developers\nwill have a better overall experience when using Git and get to focus\non what really matters to them.\n\nAs a side effect, there will be a number of other improvements:\nincreased performance, ability to rethink the design of some code that\nsuffered from patching along the time, have the chance to create new\nfeatures and fix old bugs.\n\nIn theory, switching from Bash or Shell scripts, Git will be one step\ncloser to native code which should have a positive impact on the\nperformance. Being able to start from a clean slate is a great\nopportunity to rethink old designs that may have been patched a lot\nduring their lifetime. This way, with the help of my mentors, I can\nthink of new ways to try and remove some limitations of the current\nsystem (if there are any).\n\nMoreover, I believe that the community will benefit greatly from new\nfeatures and bug fixes that I could help with. Even though this is not\nreally one of the main goals of this project, I believe that it would\nbe easier to fix bugs or implement new features while rewriting the\ncode. However, I will have to discuss with my mentors and carefully\nreview issues as I would not want to divagate from the purpose of the\nproject.\n\nAs a last point, I believe it is good to have a more homogenous\ncodebase, where the majority of the code would be written in C. This\ncould increase the number of contributions to the project as there are\nmaybe more programmers who are familiar with C, and not so much with\nPerl or shell scripting.\n\n# Deliverables\nDeliverable of this project is “git stash” completely rewritten in\nportable C code. Along with the new code, there will be some additions\nand changes brought to tests to cover any new behaviour.\n\n# Related work\nLooking over the list of the other proposed projects, I believe that\n“Convert interactive rebase to C” and “git bisect improvements” are the\nmost alike with this project and may be stretch goal of this project.\n\nMoreover, there is a chance that other scripts could benefit from this\nproject if it were to be taken as an example for future conversions.\n\n# Biographical information\nI am a freshman at Politehnica University of Bucharest, which is\nconsidered to be one of the most prestigious universities in the\nEastern Europe. I consider myself an ambitious software engineer that\nenjoys competition. This has been proven by my participation to\nprogramming competitions and extracurricular projects. As much as I\nlike competitions, I also love working with and meeting people that\nshare my interests for programming and technology.\n\nEven though, in the last two years I found myself to be more interested\nin Android software development rather than competitive programming, I\nstill take part in most of the competitions, such as contests organized\nby Google HashCode, Google KickStart and ACM.\n\nI have a good grasp of programming languages such as C and C++ and I\nconsider both of them to be my favorites. In the past, I intensively\nused shell scripting and Git for some of my personal projects.\n\nOne of the facts that motivates me the most is that I get the chance to\nimprove my abilities by getting the code reviewed by professionals that\nhave been in the industry for a long period of time. This way, I get\nfirst-hand experience regarding software management and increase my\nprogramming skills in a professional environment.\n\nUltimately, I believe that being part of a community such as Git would\nbe a great confidence boost for me knowing that I gave something back\nto an excellent piece of software that I am using on a daily basis.\n\n# Project Details\n* Plan for converting each command\n\n* Understand the command well: read documentation, discover any\nfeatures I were not aware of, find out how it works and why it works\nthat way.\n\n* Find out all edge and corner cases: these should be quite obvious as\nsoon as I have a good understanding of how the command works.\n\n* Find out the history of the command and what future improvements can\nbe made: history of the command will help me know what issues affected\nthe command, how those issues were fixed and will help me avoid them in\nthe future. As I mentioned before, I believe that reimplementing those\ncommands is a great excuse to fix bugs and if there are any, I need to\nknow about them.\n\n* Convert function: this step is basically makes up the goal of this\nproject.\n\n* Ensure that no regression occurred: considering that there are plenty\nof tests and that I have a good understanding of the function, this\nshould be a trivial task.\n\n* Finally write more tests to ensure best code coverage.\n\n# Potential difficulties during conversion\nThe rewritten code may introduce new bugs that may be delaying the\nproject. I believe there is a small chance this will happen as long as\nI spend some time first to fully understand how it works and why it\nworks that way. Considering the amount of documentation already\navailable and the discussions from the mailing list, this should be a\ntrivial task. Moreover, I am already familiar with “git stash”.\n\nThere may be old bugs, from the Shell scripts that would cause issues\nwhile porting because they would probably require a discussion about\nwhat implications the bug has and what is the best way to fix it.\n\nMost of the times, Shell scripts tend to be a lot more smaller and hide\na lot of complexity. Moreover, new data structures or utility functions\nmay be needed, some which exist and need changes or do not exist and\nneed to be implemented from scratch.\n\n# Wrapping-up the project\nIn the end, the old Shell script will serve only as a called to the\nstash-helper I would be implementing. The final step is to remove the\nShell script and promote the stash-helper to stash builtin.\n\n# Useful resources\nThere has been a lot of progress made in this direction already and I\nbelieve it will serve of great help. However, from my understanding it\nis not yet mergeable and it requires changes.\n\nhttps://public-inbox.org/git/20170608005535.13080-1-joel@teichroeb.net/\nT/#m8849c7ce0ad8516cc206dd6910b79591bf9b3acd\n\n# Project Schedule\nI can spend about 25 hours during the exam period (last half of May and\nthe first half of June) and about 40 hours a week as soon as I am done\nwith exams. I hope that by the start of the program (14th of May), I\nwill have completed a good part of the project and that will compensate\nfor the exam period.\n\nAfter Google Summer of Code ends, I will do my best to keep\ncontributing to Git. Hopefully, all the work I proposed over the summer\nwill be done and I will be able to move towards other areas, maybe by\ncontinuing converting other scripts to builtins or something else,\ndepending on what are the priorities of the Git team.\n\nGoogle Summer of Code starts on 14th of May and lasts 13 weeks. I\npropose to convert one command per week. I actually do not expect each\ncommand to take exactly one week, some may be easier to convert and\nother may be more challenging, but I believe that the average time of\nconversion is around 1 week.\n\nI am expecting to submit a patch for every command that is converted.\nThis way, I have well set milestones and results will be seen as soon\nas possible. Moreover, seeing my contributions getting merged will be a\nhuge confidence boost to myself.\n\nEach “convert” phase of the project below is not only about converting\nfrom Shell to C, but also ensuring that everything is documented, there\nare enough tests and there are no regressions.\n\n14th May - 20th May     \t- Convert \"git stash list\"\n21st May - 27th May     \t- Convert \"git stash show\"\n28th May - 3rd June     \t- Convert \"git stash save\"\n4th June - 10th June    \t- Convert \"git stash push\"\n11th June - 17th June*  \t- Convert \"git stash apply\"\n18th June - 24th June   \t- Convert \"git stash clear\"\n25th June - 1st July    \t- Convert \"git stash create\"\n2nd July - 8th July     \t- Convert \"git stash store\"\n9th July - 15th July*  \t\t- Convert \"git stash drop\"\n16th July - 22nd July    \t- Convert \"git stash pop\"\n23rd July - 29th July      \t- Convert \"git stash branch\"\n30th July - 5th August \t\t- Remove old Shell script.\n6th August - 12th August* \t- Wrap-up the project by writing more\ntests and documentation.\n12th August - forever \t\t- Pick up another project.\n\n* 1st, 2nd and 3rd mentor and student evaluation\n\n# Git contributions\nMy first contribution to Git was to help making “git tag --contains\n<id>” les chatty if <id> is invalid. Looking over the list of available\nmicroprojects, there were a couple of microprojects which got my\nattention, but I picked this up because it seemed to be a long-standing \nbug (I noticed it was approached by students in 2016, 2017 and now in\n2018). Thanks to the code reviews from the mailing list, after a few\niterations I succeeded in coming up with a patch that not only fixed\nthe mentioned problem, but also a few others that were having the same\ncause.\n\nIt got merged in the proposed updates branch.\n\nI worked on only one project because I tried to achieve the best\nquality I could. Even after I submitted one patch that was good enough\n[7], I decided to try another approach that worked better in the end\n[8].\n\nFirst of all, I weighed the ideas mentioned at [9] and considered the\nsecond one to be the best. After a few iterations [1], [2], [3], [4],\n[5] and after Junio’s review I decided to try another approach and\nsubmitted another patch [6].\n\nI changed the approach, because this issue was affecting not only “git\ntag” command, but also “git for-each-ref”, “git branch” and many more.\nThe new submission was based on Junio’s ideea and fixed the cause at a\nlower level than the previous patch.\n\nIn order to make sure that the new code was 100% correct and\nfamiliarize myself with the testing system, I also write a new set of\ntests.\n\n[1]\nhttps://public-inbox.org/git/20180219212130.4217-1-ungureanupaulsebasti\nan@gmail.com/\n\n[2]\nhttps://public-inbox.org/git/20180223162557.31477-1-ungureanupaulsebast\nian@gmail.com/\n\n[3]\nhttps://public-inbox.org/git/20180303210938.32474-1-ungureanupaulsebast\nian@gmail.com/\n\n[4]\nhttps://public-inbox.org/git/20180306193116.23876-1-ungureanupaulsebast\nian@gmail.com/\n\n[5]\nhttps://public-inbox.org/git/20180319185436.14309-1-ungureanupaulsebast\nian@gmail.com/\n\n[6]\nhttps://public-inbox.org/git/20180320175005.18759-1-ungureanupaulsebast\nian@gmail.com/T/#u\n\n[7]\nhttps://public-inbox.org/git/xmqqpo4ne8ud.fsf@gitster-ct.c.googlers.com\n/\n\n[8]\nhttps://public-inbox.org/git/xmqqefkm6s06.fsf@gitster-ct.c.googlers.com\n/\n\n[9]\nhttps://public-inbox.org/git/20160118215433.GB24136@sigill.intra.peff.n\net/\n"},{"id":"342367","messageId":"CAP8UFD3NKCSN8mVDiCUzvor5uZh4nFCAw4T0zgxpvHLf9AWmyA@mail.gmail.com","threadId":"48101","inReplyTo":"1521576562.2188.10.camel@gmail.com","subject":"Re: [GSoC] Convert “git stash” to builtin proposal","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-03-20T22:08:37Z","receivedAt":"2018-03-20T22:08:43Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nOn Tue, Mar 20, 2018 at 9:09 PM, Paul-Sebastian Ungureanu\n<ungureanupaulsebastian@gmail.com> wrote:\n>\n> * Convert function: this step is basically makes up the goal of this\n> project.\n\nCould you explain a bit more how you would convert a function? Or\ncould you explain for example how you would convert \"git stash list\"\nbelow?\n\n> * Ensure that no regression occurred: considering that there are plenty\n> of tests and that I have a good understanding of the function, this\n> should be a trivial task.\n>\n> * Finally write more tests to ensure best code coverage.\n\nMaybe, as Dscho suggested to another potential GSoC student, it would\nbe better to write more tests before converting the function.\n\n> # Useful resources\n> There has been a lot of progress made in this direction already and I\n> believe it will serve of great help. However, from my understanding it\n> is not yet mergeable and it requires changes.\n>\n> https://public-inbox.org/git/20170608005535.13080-1-joel@teichroeb.net/\n> T/#m8849c7ce0ad8516cc206dd6910b79591bf9b3acd\n\nMaybe you should Cc the author of this patch series.\n\nAlso please notice that the patch series started with adding some tests.\n\n> I am expecting to submit a patch for every command that is converted.\n> This way, I have well set milestones and results will be seen as soon\n> as possible. Moreover, seeing my contributions getting merged will be a\n> huge confidence boost to myself.\n\n> Each “convert” phase of the project below is not only about converting\n> from Shell to C, but also ensuring that everything is documented, there\n> are enough tests and there are no regressions.\n>\n> 14th May - 20th May             - Convert \"git stash list\"\n\nFor example could you explain a bit more which functions/commands you\nwould create in which file and how you would call them to convert `git\nstash list`\n"},{"id":"342549","messageId":"1521760546.11809.20.camel@gmail.com","threadId":"48101","inReplyTo":"CAP8UFD3NKCSN8mVDiCUzvor5uZh4nFCAw4T0zgxpvHLf9AWmyA@mail.gmail.com","subject":"Re: [GSoC] Convert “git stash” to builtin proposal","fromName":"Paul-Sebastian Ungureanu","fromEmail":"ungureanupaulsebastian@gmail.com","sentAt":"2018-03-22T23:15:46Z","receivedAt":"2018-03-22T23:15:56Z","isPatch":false,"sender":{"key":"ungureanupaulsebastian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/24317622?v=4"},"body":"Hello,\n\nOn Tue, 2018-03-20 at 23:08 +0100, Christian Couder wrote:\n> Hi,\n> \n> On Tue, Mar 20, 2018 at 9:09 PM, Paul-Sebastian Ungureanu\n> <ungureanupaulsebastian@gmail.com> wrote:\n> > \n> > * Convert function: this step is basically makes up the goal of\n> > this\n> > project.\n> \n> Could you explain a bit more how you would convert a function? Or\n> could you explain for example how you would convert \"git stash list\"\n> below?\n\nIn order to convert a command, all the functions which are used by the\ncommand must be converted first. The conversion will start with the\nbottom-level functions, which do not have any dependencies.\n\nFor example, to convert \"git stash list\", the parser will call\n“list_stash”, which will call “have_stash”. The conversion of these\nfunctions will be made in reverse order they were mentioned (have_stash\nfirst and then list_stash).\n\nIt is very important to know the Git source well in order to avoid\nreimplementing functionality. In this case “have_stash()” is somehow\nalready implemented as “get_oid(ref_stash, &obj)”. \n\n> > I am expecting to submit a patch for every command that is\n> > converted.\n> > This way, I have well set milestones and results will be seen as\n> > soon\n> > as possible. Moreover, seeing my contributions getting merged will\n> > be a\n> > huge confidence boost to myself.\n> > Each “convert” phase of the project below is not only about\n> > converting\n> > from Shell to C, but also ensuring that everything is documented,\n> > there\n> > are enough tests and there are no regressions.\n> > \n> > 14th May - 20th May             - Convert \"git stash list\"\n> \n> For example could you explain a bit more which functions/commands you\n> would create in which file and how you would call them to convert\n> `git\n> stash list`\n\nThe new C code will be found in stash-helper.c and will be used by git-\nstash.sh until the full conversion is complete. As soon as the entire\nconversion is done, stash-helper.c will be promoted to stash.c. Any\nfunctionality that will be implemented, but is not strictly related to\ngit stash will reside in the appropriate files (for example if I had to\nimplement similar to get_oid, which is not related to git stash, but to\nGit, then I would not implement it in stash-helper.c; anyway, I do not\nbelieve I will encounter this situation that often).\n\nIn the updated version of the proposal [1], I included the ideas stated\nbefore and more information about the procces of benchmarking. In\naddition, to test my capabilities and to be sure that I am able to work\non a project of this kind, I tried to convert “git stash list” into a\nbuilt-in (the code can be found in proposal). \n\n[1] https://docs.google.com/document/d/1TtdD7zgUsEOszHG5HX1at4zHMDsPmSB\nYWqydOPTTpV8/edit\n\nConvert “git stash” to builtin\n\n## Name and Contact Information\n------------------------------------------\nHello! My name is Paul-Sebastian Ungureanu. I am currently a first year\nComputer Science & Engineering student at Politehnica University of\nBucharest, Romania.\n\nMy email address is ungureanupaulsebastian@gmail.com and my phone\nnumber is [CENSORED]. You can also find me on #git IRC channel as\nungps.\n\nFLOSS manual recommends students to include in their proposal their\npostal address and mention a relative as a emergency contact. My\npermanent address is [CENSORED]. In case of an emergency, you may\ncontact my brother, [CENSORED] by email at [CENSORED] or by phone at\n[CENSORED].\n\n## Synopsis\n------------------------------------------\nCurrently, many components of Git are still in the form of Shell or\nPerl scripts. This choice makes sense especially when considering how\nfaster new features can be implemented in Shell and Perl scripts,\nrather than writing them in C. However, in production, where Git runs\ndaily on millions of computers with distinct configurations (i.e.\ndifferent operating systems) some problems appear (i.e. POSIX-to-\nWindows path conversion issues).\n\nThe idea of this project is to take “git-stash.sh” and reimplement it\nin C. Apart from fixing compatibility issues and increasing the\nperformance by going one step closer to native code, I believe this is\nan excellent excuse to fix long-standing bugs or implement new minor\nfeatures.\n\n## Benefits to community\n------------------------------------------\nThe essential benefit brought by rewriting Git commands is the\nincreased compatibility with a large number computers with distinct\nconfiguration. I believe that this project can have a positive impact\non a large mass of developers around the world. By rewriting the code\nbehind some popular commands and making them “built-in”, developers\nwill have a better overall experience when using Git and get to focus\non what really matters to them.\n\nAs a side effect, there will be a number of other improvements:\nincreased performance, ability to rethink the design of some code that\nsuffered from patching along the time, have the chance to create new\nfeatures and fix old bugs.\n\nIn theory, switching from Bash or Shell scripts, Git will be one step\ncloser to native code which should have a positive impact on the\nperformance. Being able to start from a clean slate is a great\nopportunity to rethink old designs that may have been patched a lot\nduring their lifetime. This way, with the help of my mentors, I can\nthink of new ways to try and remove some limitations of the current\nsystem (if there are any).\n\nMoreover, I believe that the community will benefit greatly from new\nfeatures and bug fixes that I could help with. Even though this is not\nreally one of the main goals of this project, I believe that it would\nbe easier to fix bugs or implement new features while rewriting the\ncode. However, I will have to discuss with my mentors and carefully\nreview issues as I would not want to divagate from the purpose of the\nproject.\n\nAs a last point, I believe it is good to have a more homogenous\ncodebase, where the majority of the code would be written in C. This\ncould increase the number of contributions to the project as there are\nmaybe more programmers who are familiar with C, and not so much with\nPerl or shell scripting.\n\n## Deliverables\n------------------------------------------\nDeliverable of this project is “git stash” completely rewritten in\nportable C code. Along with the new code, there will be some additions\nand changes brought to tests to cover any new behaviour.\n\n## Related work\n------------------------------------------\nLooking over the list of the other proposed projects, I believe that\n“Convert interactive rebase to C” and “git bisect improvements” are the\nmost alike with this project and may be stretch goal of this project.\n\nMoreover, there is a chance that other scripts could benefit from this\nproject if it were to be taken as an example for future conversions.\n\n## Biographical information\n------------------------------------------\nI am a freshman at Politehnica University of Bucharest, which is\nconsidered to be one of the most prestigious universities in the\nEastern Europe. I consider myself an ambitious software engineer that\nenjoys competition. This has been proven by my participation to\nprogramming competitions and extracurricular projects. As much as I\nlike competitions, I also love working with and meeting people that\nshare my interests for programming and technology.\n\nEven though, in the last two years I found myself to be more interested\nin Android software development rather than competitive programming, I\nstill take part in most of the competitions, such as contests organized\nby Google HashCode, Google KickStart and ACM.\n\nI have a good grasp of programming languages such as C and C++ and I\nconsider both of them to be my favorites. In the past, I intensively\nused shell scripting and Git for some of my personal projects.\n\nOne of the facts that motivates me the most is that I get the chance to\nimprove my abilities by getting the code reviewed by professionals that\nhave been in the industry for a long period of time. This way, I get\nfirst-hand experience regarding software management and increase my\nprogramming skills in a professional environment.\n\nUltimately, I believe that being part of a community such as Git would\nbe a great confidence boost for me knowing that I gave something back\nto an excellent piece of software that I am using on a daily basis.\n\n## Project Details\n------------------------------------------\n# Plan for converting each command\n* Understand the command well: read documentation, discover any\nfeatures I were not aware of, find out how it works and why it works\nthat way.\n\n* Find out all edge and corner cases: these should be quite obvious as\nsoon as I have a good understanding of how the command works.\n\n* Find out the history of the command and what future improvements can\nbe made: history of the command will help me know what issues affected\nthe command, how those issues were fixed and will help me avoid them in\nthe future. As I mentioned before, I believe that reimplementing those\ncommands is a great excuse to fix bugs and if there are any, I need to\nknow about them.\n\n* Write tests to cover all possible cases. My process of development is\ngoing to be similar to Test-Driven Development where a good amount of\ntests is required.\n\n* Convert function: this step basically makes up the goal of this\nproject. In order to convert a command, all the functions which are\nused by the command must be converted first. The conversion will start\nwith the bottom-level functions, which do not have any dependencies.\nThe new C code will be found in stash-helper.c and will be used by git-\nstash.sh until the full conversion is complete. As soon as the entire\nconversion is done, stash-helper.c will be promoted to stash.c. Any\nfunctionality that will be implemented, but is not strictly related to\ngit stash will reside in the appropriate files (for example if I had to\nimplement similar to get_oid, which is not related to git stash, but to\nGit, then I would not implement it in stash-helper.c; anyway, I do not\nbelieve I will encounter this situation that often).\n\n* For example, to convert \"git stash list\", the parser will call\n“list_stash”, which will call “have_stash”. The conversion of these\nfunctions will be made in reverse order they were mentioned (have_stash\nfirst and then list_stash).\n\n* It is very important to know the Git source well in order to avoid\nreimplementing functionality. In this case “have_stash()” is somehow\nalready implemented as “get_oid(ref_stash, &obj)”.\n\n* Ensure that no regression occurred: considering that there are plenty\nof tests and that I have a good understanding of the function, this\nshould be a trivial task.\n\n* Write more tests to ensure the best code coverage. This step will be\nalmost non existent due to step 4.\n\n* In the end, an analysis based on performance can be made.\nBenchmarking the script will be done by recording the running time\ngiven a large set of diversified tests that cover all the\nfunctionalities, before and after conversion. The new commands should\nrun faster just because they were written in C, but I expect to see\neven more improvements.\n\n# Potential difficulties during conversion\n------------------------------------------\n* The rewritten code may introduce new bugs that may be delaying the\nproject. I believe there is a small chance this will happen as long as\nI spend some time first to fully understand how it works and why it\nworks that way. Considering the amount of documentation already\navailable and the discussions from the mailing list, this should be a\ntrivial task. Moreover, I am already familiar with “git stash”.\n\n* There may be old bugs, from the Shell scripts that would cause issues\nwhile porting because they would probably require a discussion about\nwhat implications the bug has and what is the best way to fix it.\n\n* Most of the times, Shell scripts tend to be a lot more smaller and\nhide a lot of complexity. Moreover, new data structures or utility\nfunctions may be needed, some which exist and need changes or do not\nexist and need to be implemented from scratch.\n\n# Wrapping-up the project\n------------------------------------------\nIn the end, the old Shell script will serve only as a called to the\nstash-helper I would be implementing. The final step is to remove the\nShell script and promote the stash-helper to stash builtin.\n\n## Example of conversion (for “git stash list”)\n------------------------------------------\nTo test my capabilities and to be sure that I am able to work on a\nproject of this kind, I tried to convert “git stash list” into a built-\nin. The result can be found below in text-only version or at the Github\nlink.\n\nhttps://github.com/ungps/git/commit/3ffdece881525ee021a0f057a797e0717e1\n01094\n\nFrom 3ffdece881525ee021a0f057a797e0717e101094 Mon Sep 17 00:00:00 2001\nFrom: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>\nDate: Wed, 21 Mar 2018 20:06:23 +0200\nSubject: [PATCH] git-stash: convert git stash list from Shell script to\nC\n builtin\n\nCurrently, because git stash is not fully converted to C, I\nintroduced a new helper that will hold the converted commands.\n---\n Makefile                |  1 +\n builtin.h               |  1 +\n builtin/stash--helper.c | 52\n+++++++++++++++++++++++++++++++++++++++++++++++++\n git-stash.sh            |  7 +------\n git.c                   |  1 +\n 5 files changed, 56 insertions(+), 6 deletions(-)\n create mode 100644 builtin/stash--helper.c\n\ndiff --git a/Makefile b/Makefile\nindex a1d8775adb4b3..8ca361c57a8eb 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1020,6 +1020,7 @@ BUILTIN_OBJS += builtin/send-pack.o\n BUILTIN_OBJS += builtin/shortlog.o\n BUILTIN_OBJS += builtin/show-branch.o\n BUILTIN_OBJS += builtin/show-ref.o\n+BUILTIN_OBJS += builtin/stash--helper.o\n BUILTIN_OBJS += builtin/stripspace.o\n BUILTIN_OBJS += builtin/submodule--helper.o\n BUILTIN_OBJS += builtin/symbolic-ref.o\ndiff --git a/builtin.h b/builtin.h\nindex 42378f3aa471e..2ddb4bd5c76fa 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -220,6 +220,7 @@ extern int cmd_show(int argc, const char **argv,\nconst char *prefix);\n extern int cmd_show_branch(int argc, const char **argv, const char\n*prefix);\n extern int cmd_status(int argc, const char **argv, const char\n*prefix);\n extern int cmd_stripspace(int argc, const char **argv, const char\n*prefix);\n+extern int cmd_stash__helper(int argc, const char **argv, const char\n*prefix);\n extern int cmd_submodule__helper(int argc, const char **argv, const\nchar *prefix);\n extern int cmd_symbolic_ref(int argc, const char **argv, const char\n*prefix);\n extern int cmd_tag(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/stash--helper.c b/builtin/stash--helper.c\nnew file mode 100644\nindex 0000000000000..61fd5390d7d61\n--- /dev/null\n+++ b/builtin/stash--helper.c\n@@ -0,0 +1,52 @@\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"parse-options.h\"\n+#include \"argv-array.h\"\n+\n+enum {\n+\tLIST_STASH = 1\n+};\n+\n+static const char * ref_stash = \"refs/stash\";\n+\n+static const char * const git_stash__helper_usage[] = {\n+\tN_(\"git stash--helper --list [<options>]\"),\n+\tNULL\n+};\n+\n+static int list_stash(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct object_id obj;\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\n+\tif (get_oid(ref_stash, &obj))\n+\t\treturn 0;\n+\n+\targv_array_pushl(&args, \"log\", \"--format=%gd: %gs\", \"-g\", \"\n--first-parent\", \"-m\", NULL);\n+\targv_array_pushv(&args, argv);\n+\targv_array_push(&args, ref_stash);\n+\treturn !!cmd_log(args.argc, args.argv, prefix);\n+}\n+\n+int cmd_stash__helper(int argc, const char **argv, const char *prefix)\n+{\n+\tint cmdmode = 0;\n+\n+\tstruct option options[] = {\n+\t\tOPT_CMDMODE(0, \"list\", &cmdmode,\n+\t\t\t N_(\"list stash entries\"), LIST_STASH),\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t     git_stash__helper_usage,\nPARSE_OPT_KEEP_UNKNOWN);\n+\n+\tif (!cmdmode)\n+\t\tusage_with_options(git_stash__helper_usage, options);\n+\n+\tswitch (cmdmode) {\n+\t\tcase LIST_STASH:\n+\t\t\treturn list_stash(argc, argv, prefix);\n+\t}\n+\treturn 0;\n+}\ndiff --git a/git-stash.sh b/git-stash.sh\nindex fc8f8ae6401dd..a5b9f5fb60bba 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -380,11 +380,6 @@ have_stash () {\n \tgit rev-parse --verify --quiet $ref_stash >/dev/null\n }\n\n-list_stash () {\n-\thave_stash || return 0\n-\tgit log --format=\"%gd: %gs\" -g --first-parent -m \"$@\"\n$ref_stash --\n-}\n-\n show_stash () {\n \tALLOW_UNKNOWN_FLAGS=t\n \tassert_stash_like \"$@\"\n@@ -695,7 +690,7 @@ test -n \"$seen_non_option\" || set \"push\" \"$@\"\n case \"$1\" in\n list)\n \tshift\n-\tlist_stash \"$@\"\n+\tgit stash--helper --list \"$@\"\n \t;;\n show)\n \tshift\ndiff --git a/git.c b/git.c\nindex 96cd734f12821..6fd2ccd9a81bc 100644\n--- a/git.c\n+++ b/git.c\n@@ -466,6 +466,7 @@ static struct cmd_struct commands[] = {\n \t{ \"show-branch\", cmd_show_branch, RUN_SETUP },\n \t{ \"show-ref\", cmd_show_ref, RUN_SETUP },\n \t{ \"stage\", cmd_add, RUN_SETUP | NEED_WORK_TREE },\n+\t{ \"stash--helper\", cmd_stash__helper, RUN_SETUP },\n \t{ \"status\", cmd_status, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"stripspace\", cmd_stripspace },\n \t{ \"submodule--helper\", cmd_submodule__helper, RUN_SETUP |\nSUPPORT_SUPER_PREFIX},\n\nUseful resources\nThere has been a lot of progress made in this direction already and I\nbelieve it will serve of great help. However, from my understanding it\nis not yet mergeable and it requires changes.\n\nhttps://public-inbox.org/git/20170608005535.13080-1-joel@teichroeb.net/\nT/#m8849c7ce0ad8516cc206dd6910b79591bf9b3acd\n\n## Project Schedule\n------------------------------------------\nI can spend about 25 hours during the exam period (last half of May and\nthe first half of June) and about 40 hours a week as soon as I am done\nwith exams. I hope that by the start of the program (14th of May), I\nwill have completed a good part of the project and that will compensate\nfor the exam period.\n\nAfter Google Summer of Code ends, I will do my best to keep\ncontributing to Git. Hopefully, all the work I proposed over the summer\nwill be done and I will be able to move towards other areas, maybe by\ncontinuing converting other scripts to builtins or something else,\ndepending on what are the priorities of the Git team.\n\nGoogle Summer of Code starts on 14th of May and lasts 13 weeks. I\npropose to convert one command per week. I actually do not expect each\ncommand to take exactly one week, some may be easier to convert and\nother may be more challenging, but I believe that the average time of\nconversion is around 1 week.\n\nI am expecting to submit a patch for every command that is converted.\nThis way, I have well set milestones and results will be seen as soon\nas possible. Moreover, seeing my contributions getting merged will be a\nhuge confidence boost to myself.\n\nEach “convert” phase of the project below is not only about converting\nfrom Shell to C, but also ensuring that everything is documented, there\nare enough tests and there are no regressions.\n\n14th May - 20th May     \t- Convert \"git stash list\"\n21st May - 27th May     \t- Convert \"git stash show\"\n28th May - 3rd June     \t- Convert \"git stash save\"\n4th June - 10th June    \t- Convert \"git stash push\"\n11th June - 17th June*  \t- Convert \"git stash apply\"\n18th June - 24th June   \t- Convert \"git stash clear\"\n25th June - 1st July    \t- Convert \"git stash create\"\n2nd July - 8th July     \t- Convert \"git stash store\"\n9th July - 15th July*  \t\t- Convert \"git stash drop\"\n16th July - 22nd July    \t- Convert \"git stash pop\"\n23rd July - 29th July      \t- Convert \"git stash branch\"\n30th July - 5th August \t\t- Remove old Shell script.\n6th August - 12th August* \t- Wrap-up the project by writing more\ntests and documentation.\n12th August - forever \t\t- Pick up another project.\n\n* 1st, 2nd and 3rd mentor and student evaluation\n\n## Git contributions\n------------------------------------------\nMy first contribution to Git was to help making “git tag --contains\n<id>” les chatty if <id> is invalid. Looking over the list of available\nmicroprojects, there were a couple of microprojects which got my\nattention, but I picked this up because it seemed to be a long-standing \nbug (I noticed it was approached by students in 2016, 2017 and now in\n2018). Thanks to the code reviews from the mailing list, after a few\niterations I succeeded in coming up with a patch that not only fixed\nthe mentioned problem, but also a few others that were having the same\ncause.\n\nIt got merged in the proposed updates branch.\n\nI worked on only one project because I tried to achieve the best\nquality I could. Even after I submitted one patch that was good enough\n[8], I decided to try another approach that worked better in the end\n[9].\n\nFirst of all, I weighed the ideas mentioned at [10] and considered the\nsecond one to be the best. After a few iterations [1], [2], [3], [4],\n[5], [6] and after Junio’s review I decided to try another approach and\nsubmitted another patch [7].\n\nI changed the approach, because this issue was affecting not only “git\ntag” command, but also “git for-each-ref”, “git branch” and many more.\nThe new submission was based on Junio’s ideea and fixed the cause at a\nlower level than the previous patch.\n\nIn order to make sure that the new code was 100% correct and\nfamiliarize myself with the testing system, I also write a new set of\ntests.\n\n[1]\nhttps://public-inbox.org/git/20180219212130.4217-1-ungureanupaulsebasti\nan@gmail.com/\n\n[2]\nhttps://public-inbox.org/git/20180223162557.31477-1-ungureanupaulsebast\nian@gmail.com/\n\n[3]\nhttps://public-inbox.org/git/20180303210938.32474-1-ungureanupaulsebast\nian@gmail.com/\n\n[4]\nhttps://public-inbox.org/git/20180306193116.23876-1-ungureanupaulsebast\nian@gmail.com/\n\n[5]\nhttps://public-inbox.org/git/20180319185436.14309-1-ungureanupaulsebast\nian@gmail.com/\n\n[6]\nhttps://public-inbox.org/git/20180320175005.18759-1-ungureanupaulsebast\nian@gmail.com/\n\n[7]\nhttps://public-inbox.org/git/20180322184351.2392-1-ungureanupaulsebasti\nan@gmail.com/\n\n[8]\nhttps://public-inbox.org/git/xmqqpo4ne8ud.fsf@gitster-ct.c.googlers.com\n/\n\n[9]\nhttps://public-inbox.org/git/xmqqefkm6s06.fsf@gitster-ct.c.googlers.com\n/\n\n[10]\nhttps://public-inbox.org/git/20160118215433.GB24136@sigill.intra.peff.n\net/\n"},{"id":"342600","messageId":"nycvar.QRO.7.76.6.1803231754390.77@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48101","inReplyTo":"1521760546.11809.20.camel@gmail.com","subject":"Re: [GSoC] Convert “git stash” to builtin proposal","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-23T17:06:17Z","receivedAt":"2018-03-23T17:06:36Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul-Sebastian,\n\nOn Fri, 23 Mar 2018, Paul-Sebastian Ungureanu wrote:\n\n> On Tue, 2018-03-20 at 23:08 +0100, Christian Couder wrote:\n> > Hi,\n> > \n> > On Tue, Mar 20, 2018 at 9:09 PM, Paul-Sebastian Ungureanu\n> > <ungureanupaulsebastian@gmail.com> wrote:\n> > > \n> > > * Convert function: this step is basically makes up the goal of\n> > > this\n> > > project.\n> > \n> > Could you explain a bit more how you would convert a function? Or\n> > could you explain for example how you would convert \"git stash list\"\n> > below?\n> \n> In order to convert a command, all the functions which are used by the\n> command must be converted first. The conversion will start with the\n> bottom-level functions, which do not have any dependencies.\n> \n> For example, to convert \"git stash list\", the parser will call\n> “list_stash”, which will call “have_stash”. The conversion of these\n> functions will be made in reverse order they were mentioned (have_stash\n> first and then list_stash).\n> \n> It is very important to know the Git source well in order to avoid\n> reimplementing functionality. In this case “have_stash()” is somehow\n> already implemented as “get_oid(ref_stash, &obj)”. \n\nVery good\n\n> [... proposal ...]\n\nThis is a pretty good proposal. The initial draft at converting `stash\nlist` is a good start (it will need to be converted to avoid spawning an\nextra process, but that is something we can do incrementally, together).\n\nCiao,\nJohannes"},{"id":"342603","messageId":"CAP8UFD3bRaPke8MvubZ3+v6RrY7K7Peip1dpQ2LG9kxKoXcmbw@mail.gmail.com","threadId":"48101","inReplyTo":"1521760546.11809.20.camel@gmail.com","subject":"Re: [GSoC] Convert “git stash” to builtin proposal","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-03-23T17:11:34Z","receivedAt":"2018-03-23T17:11:42Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Mar 23, 2018 at 12:15 AM, Paul-Sebastian Ungureanu\n<ungureanupaulsebastian@gmail.com> wrote:\n>\n> On Tue, 2018-03-20 at 23:08 +0100, Christian Couder wrote:\n>>\n>> On Tue, Mar 20, 2018 at 9:09 PM, Paul-Sebastian Ungureanu\n>> <ungureanupaulsebastian@gmail.com> wrote:\n>> >\n>> > * Convert function: this step is basically makes up the goal of\n>> > this\n>> > project.\n>>\n>> Could you explain a bit more how you would convert a function? Or\n>> could you explain for example how you would convert \"git stash list\"\n>> below?\n>\n> In order to convert a command, all the functions which are used by the\n> command must be converted first. The conversion will start with the\n> bottom-level functions, which do not have any dependencies.\n>\n> For example, to convert \"git stash list\", the parser will call\n> “list_stash”, which will call “have_stash”. The conversion of these\n> functions will be made in reverse order they were mentioned (have_stash\n> first and then list_stash).\n\nOk.\n\n> It is very important to know the Git source well in order to avoid\n> reimplementing functionality. In this case “have_stash()” is somehow\n> already implemented as “get_oid(ref_stash, &obj)”.\n>\n>> > I am expecting to submit a patch for every command that is\n>> > converted.\n>> > This way, I have well set milestones and results will be seen as\n>> > soon\n>> > as possible. Moreover, seeing my contributions getting merged will\n>> > be a\n>> > huge confidence boost to myself.\n>> > Each “convert” phase of the project below is not only about\n>> > converting\n>> > from Shell to C, but also ensuring that everything is documented,\n>> > there\n>> > are enough tests and there are no regressions.\n>> >\n>> > 14th May - 20th May             - Convert \"git stash list\"\n>>\n>> For example could you explain a bit more which functions/commands you\n>> would create in which file and how you would call them to convert\n>> `git\n>> stash list`\n>\n> The new C code will be found in stash-helper.c and will be used by git-\n> stash.sh until the full conversion is complete. As soon as the entire\n> conversion is done, stash-helper.c will be promoted to stash.c. Any\n> functionality that will be implemented, but is not strictly related to\n> git stash will reside in the appropriate files (for example if I had to\n> implement similar to get_oid, which is not related to git stash, but to\n> Git, then I would not implement it in stash-helper.c; anyway, I do not\n> believe I will encounter this situation that often).\n\nOk.\n\n> In the updated version of the proposal [1], I included the ideas stated\n> before and more information about the procces of benchmarking. In\n> addition, to test my capabilities and to be sure that I am able to work\n> on a project of this kind, I tried to convert “git stash list” into a\n> built-in (the code can be found in proposal).\n>\n> [1] https://docs.google.com/document/d/1TtdD7zgUsEOszHG5HX1at4zHMDsPmSB\n> YWqydOPTTpV8/edit\n\nI guess that below you inlined the updated version of your proposal. Nice.\n\n> Convert “git stash” to builtin\n\n[...]\n\n> * Ensure that no regression occurred: considering that there are plenty\n> of tests and that I have a good understanding of the function, this\n> should be a trivial task.\n\nThere are a lot of things that the test suite doesn't test.\n\n> * Write more tests to ensure the best code coverage. This step will be\n> almost non existent due to step 4.\n\nMaybe you should use a numbered list if you want to refer to the items\nby their number.\n\n> * In the end, an analysis based on performance can be made.\n> Benchmarking the script will be done by recording the running time\n> given a large set of diversified tests that cover all the\n> functionalities, before and after conversion. The new commands should\n> run faster just because they were written in C, but I expect to see\n> even more improvements.\n\nIf you want to do benchmarks, you might want to add performance tests\ninto t/perf.\n\n[...]\n\n> ## Example of conversion (for “git stash list”)\n> ------------------------------------------\n> To test my capabilities and to be sure that I am able to work on a\n> project of this kind, I tried to convert “git stash list” into a built-\n> in. The result can be found below in text-only version or at the Github\n> link.\n\nI think it would be better if it was sent as a patch (maybe an RFC\npatch) to the mailing list and add the link to the patch email in the\nmaling list archive to your proposal.\n\n[...]\n\n> Useful resources\n> There has been a lot of progress made in this direction already and I\n> believe it will serve of great help. However, from my understanding it\n> is not yet mergeable and it requires changes.\n>\n> https://public-inbox.org/git/20170608005535.13080-1-joel@teichroeb.net/\n> T/#m8849c7ce0ad8516cc206dd6910b79591bf9b3acd\n\nIt could be useful if you described a bit more how you could (re)use\nthe work that has already been made.\n\nIn the rest of your proposal it looks like you are starting from\nscratch, which is a bit strange if a lot of progress has already been\nmade.\n\n> ## Project Schedule\n> ------------------------------------------\n> I can spend about 25 hours during the exam period (last half of May and\n> the first half of June) and about 40 hours a week as soon as I am done\n> with exams. I hope that by the start of the program (14th of May), I\n> will have completed a good part of the project and that will compensate\n> for the exam period.\n>\n> After Google Summer of Code ends, I will do my best to keep\n> contributing to Git. Hopefully, all the work I proposed over the summer\n> will be done and I will be able to move towards other areas, maybe by\n> continuing converting other scripts to builtins or something else,\n> depending on what are the priorities of the Git team.\n>\n> Google Summer of Code starts on 14th of May and lasts 13 weeks. I\n> propose to convert one command per week. I actually do not expect each\n> command to take exactly one week, some may be easier to convert and\n> other may be more challenging, but I believe that the average time of\n> conversion is around 1 week.\n>\n> I am expecting to submit a patch for every command that is converted.\n> This way, I have well set milestones and results will be seen as soon\n> as possible. Moreover, seeing my contributions getting merged will be a\n> huge confidence boost to myself.\n>\n> Each “convert” phase of the project below is not only about converting\n> from Shell to C, but also ensuring that everything is documented, there\n> are enough tests and there are no regressions.\n>\n> 14th May - 20th May             - Convert \"git stash list\"\n> 21st May - 27th May             - Convert \"git stash show\"\n> 28th May - 3rd June             - Convert \"git stash save\"\n> 4th June - 10th June            - Convert \"git stash push\"\n> 11th June - 17th June*          - Convert \"git stash apply\"\n> 18th June - 24th June           - Convert \"git stash clear\"\n> 25th June - 1st July            - Convert \"git stash create\"\n> 2nd July - 8th July             - Convert \"git stash store\"\n> 9th July - 15th July*           - Convert \"git stash drop\"\n> 16th July - 22nd July           - Convert \"git stash pop\"\n> 23rd July - 29th July           - Convert \"git stash branch\"\n> 30th July - 5th August          - Remove old Shell script.\n> 6th August - 12th August*       - Wrap-up the project by writing more\n> tests and documentation.\n> 12th August - forever           - Pick up another project.\n>\n> * 1st, 2nd and 3rd mentor and student evaluation\n\nIt is probably better especially for reviewers and more common to work\nin small batches, for example to send a patch series converting a few\nrelated commands, then to start working on the next small batch of\ncommands in another patch series while improving the first patch\nseries according to reviews.\n\nAlso we ask GSoC students to communicate publicly every week about\ntheir project for example by writing a blg post or sending a report to\nthe mailing list.\n\n> ## Git contributions\n> ------------------------------------------\n> My first contribution to Git was to help making “git tag --contains\n> <id>” les chatty if <id> is invalid. Looking over the list of available\n> microprojects, there were a couple of microprojects which got my\n> attention, but I picked this up because it seemed to be a long-standing\n> bug (I noticed it was approached by students in 2016, 2017 and now in\n> 2018). Thanks to the code reviews from the mailing list, after a few\n> iterations I succeeded in coming up with a patch that not only fixed\n> the mentioned problem, but also a few others that were having the same\n> cause.\n>\n> It got merged in the proposed updates branch.\n\nWhat is its status in Junio's \"What's cooking in Git\" emails?\n\n> I worked on only one project because I tried to achieve the best\n> quality I could. Even after I submitted one patch that was good enough\n> [8], I decided to try another approach that worked better in the end\n> [9].\n>\n> First of all, I weighed the ideas mentioned at [10] and considered the\n> second one to be the best. After a few iterations [1], [2], [3], [4],\n> [5], [6] and after Junio’s review I decided to try another approach and\n> submitted another patch [7].\n>\n> I changed the approach, because this issue was affecting not only “git\n> tag” command, but also “git for-each-ref”, “git branch” and many more.\n> The new submission was based on Junio’s ideea and fixed the cause at a\n> lower level than the previous patch.\n>\n> In order to make sure that the new code was 100% correct and\n> familiarize myself with the testing system, I also write a new set of\n> tests.\n\nThanks.\n"},{"id":"342838","messageId":"6603149f-776a-fde8-5d11-a7d9d6d37e96@gmail.com","threadId":"48101","inReplyTo":"CAP8UFD3bRaPke8MvubZ3+v6RrY7K7Peip1dpQ2LG9kxKoXcmbw@mail.gmail.com","subject":"Re: [GSoC] Convert “git stash” to builtin proposal","fromName":"Paul-Sebastian Ungureanu","fromEmail":"ungureanupaulsebastian@gmail.com","sentAt":"2018-03-24T19:31:14Z","receivedAt":"2018-03-24T19:31:23Z","isPatch":false,"sender":{"key":"ungureanupaulsebastian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/24317622?v=4"},"body":"On 23.03.2018 19:11, Christian Couder wrote:\n\n>> * Ensure that no regression occurred: considering that there are plenty\n>> of tests and that I have a good understanding of the function, this\n>> should be a trivial task.\n> \n> There are a lot of things that the test suite doesn't test.\n\nHopefully, by first adding new tests, any eventual bug will be spotted.\n\n>> * In the end, an analysis based on performance can be made.\n>> Benchmarking the script will be done by recording the running time\n>> given a large set of diversified tests that cover all the\n>> functionalities, before and after conversion. The new commands should\n>> run faster just because they were written in C, but I expect to see\n>> even more improvements.\n> \n> If you want to do benchmarks, you might want to add performance tests\n> into t/perf.\n\nThat is great. I will surely make use of the existent testing framework \nto do benchmarks.\n\n>> ## Example of conversion (for “git stash list”)\n>> ------------------------------------------\n>> To test my capabilities and to be sure that I am able to work on a\n>> project of this kind, I tried to convert “git stash list” into a built-\n>> in. The result can be found below in text-only version or at the Github\n>> link.\n> \n> I think it would be better if it was sent as a patch (maybe an RFC\n> patch) to the mailing list and add the link to the patch email in the\n> maling list archive to your proposal.\n\nI sent it as a [RFC] patch to the mailing list and included it in the \nproposal.\n\n<https://public-inbox.org/git/20180324182313.13705-1-ungureanupaulsebastian@gmail.com>\n\n> It could be useful if you described a bit more how you could (re)use\n> the work that has already been made.\n> \n> In the rest of your proposal it looks like you are starting from\n> scratch, which is a bit strange if a lot of progress has already been\n> made.\n\nThe patches about converting ‘git stash’ are a great starting point and \nI will definitely use them. Each time I start converting a new command I \nwill first take a look at what other patches there are, evaluate them \nand if I consider the patch to be good enough I will continue my work on \ntop of that patch. Otherwise, I will start from scratch and that patch \nwill only serve for comparison.\n\nOne other resource that is of great importance is git itself. I can \nlearn how a builtin is structured and how it is built by considering \nexamples the other Git commands. Having a similar coding standard used, \nthe codebase will be homogeneous and hopefully easier to maintain.\n\nAnother important resource are commands that are Google Summer of Code \nprojects from previous years (2016 and 2017) which had as purpose to \nconvert “git bisect” and “git submodule”. I can always take a look at \nthe patches they submitted and read their code reviews to avoid making \nsame mistakes they did.\n\n> It is probably better especially for reviewers and more common to work\n> in small batches, for example to send a patch series converting a few\n> related commands, then to start working on the next small batch of\n> commands in another patch series while improving the first patch\n> series according to reviews.\n> \n> Also we ask GSoC students to communicate publicly every week about\n> their project for example by writing a blg post or sending a report to\n> the mailing list.\n\nNoted.\n\n>> ## Git contributions\n>> ------------------------------------------\n>> My first contribution to Git was to help making “git tag --contains\n>> <id>” les chatty if <id> is invalid. Looking over the list of available\n>> microprojects, there were a couple of microprojects which got my\n>> attention, but I picked this up because it seemed to be a long-standing\n>> bug (I noticed it was approached by students in 2016, 2017 and now in\n>> 2018). Thanks to the code reviews from the mailing list, after a few\n>> iterations I succeeded in coming up with a patch that not only fixed\n>> the mentioned problem, but also a few others that were having the same\n>> cause.\n>>\n>> It got merged in the proposed updates branch.\n> \n> What is its status in Junio's \"What's cooking in Git\" emails?\n\nIt is now in the ‘next’ branch of the Git repository.\n\nI updated the proposal, in which I included these ideas and some \nadditional examples. Thank you a lot!\n\nBest regards,\nPaul Ungureanu\n"},{"id":"342879","messageId":"CAP8UFD2y605FQeiymO2JNxy7MXs=-vRcN-Z2ri1=ttDF8kccvQ@mail.gmail.com","threadId":"48101","inReplyTo":"6603149f-776a-fde8-5d11-a7d9d6d37e96@gmail.com","subject":"Re: [GSoC] Convert “git stash” to builtin proposal","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-03-25T09:46:07Z","receivedAt":"2018-03-25T09:46:14Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Mar 24, 2018 at 8:31 PM, Paul-Sebastian Ungureanu\n<ungureanupaulsebastian@gmail.com> wrote:\n> On 23.03.2018 19:11, Christian Couder wrote:\n>\n>>> * Ensure that no regression occurred: considering that there are plenty\n>>> of tests and that I have a good understanding of the function, this\n>>> should be a trivial task.\n>>\n>> There are a lot of things that the test suite doesn't test.\n>\n> Hopefully, by first adding new tests, any eventual bug will be spotted.\n\nI was thinking about things like memory leaks that tests cannot easily spot.\n\n>>> * In the end, an analysis based on performance can be made.\n>>> Benchmarking the script will be done by recording the running time\n>>> given a large set of diversified tests that cover all the\n>>> functionalities, before and after conversion. The new commands should\n>>> run faster just because they were written in C, but I expect to see\n>>> even more improvements.\n>>\n>> If you want to do benchmarks, you might want to add performance tests\n>> into t/perf.\n>\n> That is great. I will surely make use of the existent testing framework to\n> do benchmarks.\n\nGood.\n\n>>> ## Example of conversion (for “git stash list”)\n>>> ------------------------------------------\n>>> To test my capabilities and to be sure that I am able to work on a\n>>> project of this kind, I tried to convert “git stash list” into a built-\n>>> in. The result can be found below in text-only version or at the Github\n>>> link.\n>>\n>> I think it would be better if it was sent as a patch (maybe an RFC\n>> patch) to the mailing list and add the link to the patch email in the\n>> maling list archive to your proposal.\n>\n> I sent it as a [RFC] patch to the mailing list and included it in the\n> proposal.\n>\n> <https://public-inbox.org/git/20180324182313.13705-1-ungureanupaulsebastian@gmail.com>\n\nNice.\n\n>> It could be useful if you described a bit more how you could (re)use\n>> the work that has already been made.\n>>\n>> In the rest of your proposal it looks like you are starting from\n>> scratch, which is a bit strange if a lot of progress has already been\n>> made.\n>\n> The patches about converting ‘git stash’ are a great starting point and I\n> will definitely use them. Each time I start converting a new command I will\n> first take a look at what other patches there are, evaluate them and if I\n> consider the patch to be good enough I will continue my work on top of that\n> patch. Otherwise, I will start from scratch and that patch will only serve\n> for comparison.\n>\n> One other resource that is of great importance is git itself. I can learn\n> how a builtin is structured and how it is built by considering examples the\n> other Git commands. Having a similar coding standard used, the codebase will\n> be homogeneous and hopefully easier to maintain.\n>\n> Another important resource are commands that are Google Summer of Code\n> projects from previous years (2016 and 2017) which had as purpose to convert\n> “git bisect” and “git submodule”. I can always take a look at the patches\n> they submitted and read their code reviews to avoid making same mistakes\n> they did.\n\nNice.\n\n>> It is probably better especially for reviewers and more common to work\n>> in small batches, for example to send a patch series converting a few\n>> related commands, then to start working on the next small batch of\n>> commands in another patch series while improving the first patch\n>> series according to reviews.\n>>\n>> Also we ask GSoC students to communicate publicly every week about\n>> their project for example by writing a blg post or sending a report to\n>> the mailing list.\n>\n> Noted.\n>\n>>> ## Git contributions\n>>> ------------------------------------------\n>>> My first contribution to Git was to help making “git tag --contains\n>>> <id>” les chatty if <id> is invalid. Looking over the list of available\n>>> microprojects, there were a couple of microprojects which got my\n>>> attention, but I picked this up because it seemed to be a long-standing\n>>> bug (I noticed it was approached by students in 2016, 2017 and now in\n>>> 2018). Thanks to the code reviews from the mailing list, after a few\n>>> iterations I succeeded in coming up with a patch that not only fixed\n>>> the mentioned problem, but also a few others that were having the same\n>>> cause.\n>>>\n>>> It got merged in the proposed updates branch.\n>>\n>> What is its status in Junio's \"What's cooking in Git\" emails?\n>\n> It is now in the ‘next’ branch of the Git repository.\n>\n> I updated the proposal, in which I included these ideas and some additional\n> examples. Thank you a lot!\n\nOk. Feel free to resend another version of your proposal.\n\nThanks.\n"},{"id":"342891","messageId":"30151c63-9437-e1e7-dd89-585e35232455@gmail.com","threadId":"48101","inReplyTo":"nycvar.QRO.7.76.6.1803231754390.77@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [GSoC] Convert “git stash” to builtin proposal","fromName":"Paul-Sebastian Ungureanu","fromEmail":"ungureanupaulsebastian@gmail.com","sentAt":"2018-03-25T14:32:37Z","receivedAt":"2018-03-25T14:32:45Z","isPatch":false,"sender":{"key":"ungureanupaulsebastian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/24317622?v=4"},"body":"On 23.03.2018 19:06, Johannes Schindelin wrote:\n>> [... proposal ...]\n> \n> This is a pretty good proposal. The initial draft at converting `stash\n> list` is a good start (it will need to be converted to avoid spawning an\n> extra process, but that is something we can do incrementally, together).\n\nThank you for your kind words. It feels good to see other people \nappreciate my work. It is a strong incentive to keep going on. I made a \nfew adjustments and I hope that the final version will be better.\n\nBest regards,\nPaul Ungureanu\n"},{"id":"342892","messageId":"ad524ce4-8be2-4d14-44cc-2fbc9ff60b79@gmail.com","threadId":"48101","inReplyTo":"CAP8UFD2y605FQeiymO2JNxy7MXs=-vRcN-Z2ri1=ttDF8kccvQ@mail.gmail.com","subject":"Re: [GSoC] Convert “git stash” to builtin proposal","fromName":"Paul-Sebastian Ungureanu","fromEmail":"ungureanupaulsebastian@gmail.com","sentAt":"2018-03-25T14:38:21Z","receivedAt":"2018-03-25T14:38:29Z","isPatch":false,"sender":{"key":"ungureanupaulsebastian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/24317622?v=4"},"body":"On 25.03.2018 12:46, Christian Couder wrote:\n> On Sat, Mar 24, 2018 at 8:31 PM, Paul-Sebastian Ungureanu\n> <ungureanupaulsebastian@gmail.com> wrote:\n>> On 23.03.2018 19:11, Christian Couder wrote:\n>>\n>>>> * Ensure that no regression occurred: considering that there are plenty\n>>>> of tests and that I have a good understanding of the function, this\n>>>> should be a trivial task.\n>>>\n>>> There are a lot of things that the test suite doesn't test.\n>>\n>> Hopefully, by first adding new tests, any eventual bug will be spotted.\n> \n> I was thinking about things like memory leaks that tests cannot easily spot.> \n\nI will do my best and follow best practices in order to avoid any memory \nleaks. However, to make sure that my code is really leak free, I will \nuse Valgrind, which is already integrated with the testing framework.\n\n> Ok. Feel free to resend another version of your proposal.\nSorry for not sending the whole proposal again. I decided to send only \nthe changed parts because the proposal is quite big and I wanted to \navoid sending the same thing over and over again.\n\nOne thing I did not mention in the previous reply was that I also added \na new paragraph to \"Benefits to community\" about 'git stash' being slow \non Windows for a lot of users. I consider this alone to be a very good \njustification for this project and doing this project will be very \nbeneficial for the Windows users.\n\nBest regards,\nPaul Ungureanu\n"},{"id":"343078","messageId":"nycvar.QRO.7.76.6.1803270004300.77@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48101","inReplyTo":"ad524ce4-8be2-4d14-44cc-2fbc9ff60b79@gmail.com","subject":"Re: [GSoC] Convert “git stash” to builtin proposal","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-26T22:04:59Z","receivedAt":"2018-03-26T22:05:05Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn Sun, 25 Mar 2018, Paul-Sebastian Ungureanu wrote:\n\n> One thing I did not mention in the previous reply was that I also added\n> a new paragraph to \"Benefits to community\" about 'git stash' being slow\n> on Windows for a lot of users. I consider this alone to be a very good\n> justification for this project and doing this project will be very\n> beneficial for the Windows users.\n\nYesssss!\n\nThank you,\nJohannes\n"},{"id":"343109","messageId":"CAP8UFD2Rx2Ykc1VNfqRy=UduNb5n6rQzrwTnZW7LZb1hvcBocA@mail.gmail.com","threadId":"48101","inReplyTo":"ad524ce4-8be2-4d14-44cc-2fbc9ff60b79@gmail.com","subject":"Re: [GSoC] Convert “git stash” to builtin proposal","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-03-27T06:14:35Z","receivedAt":"2018-03-27T06:14:42Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sun, Mar 25, 2018 at 4:38 PM, Paul-Sebastian Ungureanu\n<ungureanupaulsebastian@gmail.com> wrote:\n> On 25.03.2018 12:46, Christian Couder wrote:\n>>\n>> On Sat, Mar 24, 2018 at 8:31 PM, Paul-Sebastian Ungureanu\n>> <ungureanupaulsebastian@gmail.com> wrote:\n>>>\n>>> On 23.03.2018 19:11, Christian Couder wrote:\n>>>\n>>>>> * Ensure that no regression occurred: considering that there are plenty\n>>>>> of tests and that I have a good understanding of the function, this\n>>>>> should be a trivial task.\n>>>>\n>>>> There are a lot of things that the test suite doesn't test.\n>>>\n>>> Hopefully, by first adding new tests, any eventual bug will be spotted.\n>>\n>> I was thinking about things like memory leaks that tests cannot easily\n>> spot.>\n>\n> I will do my best and follow best practices in order to avoid any memory\n> leaks. However, to make sure that my code is really leak free, I will use\n> Valgrind, which is already integrated with the testing framework.\n\nYeah, but it's still not so easy even with valgrind for a number of\nreasons. For example it's difficult to test all the error paths, and\nthere are also some memory leaks that we accept when we know that they\nhappen just once and that we are going to exit soon anyway when we\nshould free the memory.\n\n>> Ok. Feel free to resend another version of your proposal.\n>\n> Sorry for not sending the whole proposal again. I decided to send only the\n> changed parts because the proposal is quite big and I wanted to avoid\n> sending the same thing over and over again.\n\nIt's up to you, but but fewer people might review it.\n\n> One thing I did not mention in the previous reply was that I also added a\n> new paragraph to \"Benefits to community\" about 'git stash' being slow on\n> Windows for a lot of users. I consider this alone to be a very good\n> justification for this project and doing this project will be very\n> beneficial for the Windows users.\n\nSure.\n"}]}