{"thread":{"id":"48094","subject":"[RFC] [GSoC] Project proposal: convert scripts to builtins","startedAt":"2018-03-20T09:04:57Z","lastAt":"2018-03-24T10:39:13Z","messageCount":6,"participants":["Pratik Karki","Christian Couder","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"342299","messageId":"20180320090013.26512-1-predatoramigo@gmail.com","threadId":"48094","inReplyTo":null,"subject":"[RFC] [GSoC] Project proposal: convert scripts to builtins","fromName":"Pratik Karki","fromEmail":"predatoramigo@gmail.com","sentAt":"2018-03-20T09:00:13Z","receivedAt":"2018-03-20T09:04:57Z","isPatch":false,"sender":{"key":"predatoramigo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12194950?v=4"},"body":"Hi,\nThis is my draft for my proposal on \"Convert Scripts to builtin\" for GSoC.\nPlease review and provide feedbacks.\n\nhttps://gist.github.com/prertik/daaa73a39d3ce30811d9a208043dc235\n\nCheers,\nPratik Karki\n"},{"id":"342300","messageId":"CAP8UFD2gN9Litjm_Wn8XoR7QMg6BXasETUX33ntcB2_gQ5AVTg@mail.gmail.com","threadId":"48094","inReplyTo":"20180320090013.26512-1-predatoramigo@gmail.com","subject":"Re: [RFC] [GSoC] Project proposal: convert scripts to builtins","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-03-20T09:16:44Z","receivedAt":"2018-03-20T09:16:49Z","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 10:00 AM, Pratik Karki <predatoramigo@gmail.com> wrote:\n> Hi,\n> This is my draft for my proposal on \"Convert Scripts to builtin\" for GSoC.\n> Please review and provide feedbacks.\n>\n> https://gist.github.com/prertik/daaa73a39d3ce30811d9a208043dc235\n\nIt would be easier for us to comment if the markdown was sent inline.\n\nThanks,\nChristian.\n"},{"id":"342301","messageId":"20180320093421.26817-1-predatoramigo@gmail.com","threadId":"48094","inReplyTo":"CAP8UFD2gN9Litjm_Wn8XoR7QMg6BXasETUX33ntcB2_gQ5AVTg@mail.gmail.com","subject":"Re: [RFC] [GSoC] Project proposal: convert scripts to builtins","fromName":"Pratik Karki","fromEmail":"predatoramigo@gmail.com","sentAt":"2018-03-20T09:34:21Z","receivedAt":"2018-03-20T09:34:58Z","isPatch":false,"sender":{"key":"predatoramigo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12194950?v=4"},"body":"Hi,\nThis is my draft for my proposal on \"Convert Scripts to builtins\" for GSoC.\nPlease review and provide feedback.\n\n\nCheers,\nPratik Karki\n\n\nConvert Scripts to builtins\n===========================\n\nAbstract\n--------\n\nMany components of Git are still in the form of shell and Perl scripts.\nThis has certain advantages of being extensible but causes problems in\nproduction code on multiple platforms like Windows.\nI propose to rewrite a couple of shell and perl scripts into portable\nand performant C code, making them built-ins. The major advantage of\ndoing this is improvement in efficiency and performance.\n\nMuch more scripts like `git-am` , `git-pull`, `git-branch` have already\nbeen rewritten in C. Much more scripts like `git-rebase`, `git-stash`,\n`git-add --interactive` are still present in shell and perl scripts.\nI propose to work in `git-rebase` and `git-stash`.\n\nShell Scripts:\n--------------\n\nAlthough shell scripts are more faster can be extensible in\nfunctionality and can be more easier to write, they introduce certain\ndisadvantages.\n\n1.  Dependencies:\n     The scripting languages and shell scripts make more productive code\n    but there is an overhead of dependencies. The shell scripts are\n    lighter and simpler and call other executables to perform\n    non-trivial tasks. Taking `git-stash` shell script for example.\n    `sed`, `rm`, `echo`, `test` are constantly present in `git-stash`.\n    These look common to POSIX platforms but for non-POSIX platforms\n    there needs some extra work for porting these commands. For example,\n    in Git for Windows, the workaround for these commands in non-POSIX\n    platform adds some extra utilities and adds MSYS2 shell commands and\n    needs to pack language runtime for Perl. This increases the\n    installation size and requires more disk space. Again, adding more\n    batteries again needs implementation in all of the dependency\n    libraries and executables.\n\n2.  Inefficiency:\n     Git has internal caches for configuration values, the repository\n    index and repository objects. The porcelain commands do not have\n    access to git's internal API and so they spawn git processes to\n    perform git operations. For every git invocation, git would re-read\n    the user's configuration files, repository index, repopulate the\n    filesystem cache, etc. This leads to overhead and unnecessary I/O.\n    Windows is known to have worse I/O performance compared to Linux.\n    There is also slower I/O performance of HDD compared to SSD. This\n    unnecessary I/O operations causes runtime overhead and becomes\n    slower in poor I/O performance setups. Now, writing the porcelain\n    into C built-ins leverages the git API and there is no need of\n    spawning separate git processes, caching can be used to reduce\n    unnecessary I/O processes.\n\n3.  Spawing processes is less performant:\n     Shell scripts usually spawn a lot of processes. Shell scripts are\n    very lighter and hence have limited functionalites. For\n    `git-stash.sh` to work it needs to perform lots of git operations\n    like `git rev-parse` `git config` and thus spawns git executable\n    processes for performing these operations. Again for invoking\n    `git config` and providing configuration values, it spawn new\n    processes to handle that. Spawning is implemented by `fork()` and\n    `exec()` by shells. Now, on systems that do not support\n    copy-on-write semantics for `fork()`, there is duplication of the\n    memory of the parent process for every `fork()` call which turns out\n    to be an expensive process. Now, in Windows, Git uses MSYS2\n    exclusively to emulate `fork()` but since, Windows doesnot support\n    forking semantics natively, the workaround provided by MSYS2\n    emulates `fork()` without [copy-on-write\n    semantics](https://www.cygwin.com/faq.html#faq.api.fork). Doing this\n    creates another layer over Windows processes and thus slows git.\n\nRewriting C built-ins\n---------------------\n\nThese above mentioned problems need to be fixed. The only fix for these\nproblems would be to write built-ins in C for all these shell scripts\nleveraging the git API. Writing in built-in reduces the dependency\nrequired by shell scripts. Since, Git is native executable in Windows,\ndoing this can make MSYS2 POSIX emulation obsolete. Then, using git's\ninternal API and C data types, built-in `git_config_get_value()` can be\nused to get configuration value rather than spawning another git-config\nprocess. This removes the necessary to re-read git configuration cache\neverytime and reduces I/O. Furthermore, git-stash and git-rebase will be\nmore faster and show consistent behaviour as instead of spawing another\nprocess and parsing command-line arguments manually, they can be\nhardcoded to be built-in and leverage all the required git's internal\nAPI's like `parse-options`.\n\nTo implement git-stash and git-rebase in C, I propose to avoid spawning\nlots of external git processes and reduce redundant I/O by taking\nadvantage of the internal object, index and configuration cache. I\npropose to use C data structures instead of needless parsing as C has\nricher and performant data structures. Windows will definitely benefit\nfrom these rewrites as the use of MSYS2 POSIX emulation would be\nobsolete and unnecessary I/O processes would be reduced and there won't\nbe need of spawning external git processes.\n\nBut, while rewriting regressions should be totally avoided. The code\nshould not cause regression and introduce bugs not present in previous\nscripts. Thought the re-write wouldn't look more simpler like previous\nscripts, it should not have different behaviour or bugs compared to the\nscript and should not be hard to maintain compared to the script.\n\nPotential Problems\n------------------\n\n### New Re-write code becomes less simpler and Bugs could get introduced:\n\nC is a more verbose language than shell script and looks less simpler\nthan shell scripts. Doing things in less than 4 lines of shell script in\nC can take up more than 12 lines to make it more simpler. For example,\nin a previously used script `git-commit.sh` we can see,\n\n```bash\n     run_status () {\n\t# If TMP_INDEX is defined, that means we are doing\n\t# \"--only\" partial commit, and that index file is used\n\t# to build the tree for the commit.  Otherwise, if\n\t# NEXT_INDEX exists, that is the index file used to\n\t# make the commit.  Otherwise we are using as-is commit\n\t# so the regular index file is what we use to compare.\n\tif test '' != \"$TMP_INDEX\"\n\tthen\n\t\tGIT_INDEX_FILE=\"$TMP_INDEX\"\n\t\texport GIT_INDEX_FILE\n\telif test -f \"$NEXT_INDEX\"\n\tthen\n\t\tGIT_INDEX_FILE=\"$NEXT_INDEX\"\n\t\texport GIT_INDEX_FILE\n\tfi\n\n\tif test \"$status_only\" = \"t\" -o \"$use_status_color\" = \"t\"; then\n\t\tcolor=\n\telse\n\t\tcolor=--nocolor\n\tfi\n\tgit runstatus ${color} \\\n\t\t${verbose:+--verbose} \\\n\t\t${amend:+--amend} \\\n\t\t${untracked_files:+--untracked}\n    }\n ```\nThis script has been rewritten as builtin in C.\nThese lines of code have been replaced in C providing more\nbuilt-in performance but looks less simpler and\ncontains high multiline functions in `commit.c`.\n\n```C\n`static int run_status(FILE *fp, const char *index_file, const char *prefix, int nowarn,\n\t\t      struct wt_status *s)\n{\n\tstruct object_id oid;\n\n\tif (s->relative_paths)\n\t\ts->prefix = prefix;\n\n\tif (amend) {\n\t\ts->amend = 1;\n\t\ts->reference = \"HEAD^1\";\n\t}\n\ts->verbose = verbose;\n\ts->index_file = index_file;\n\ts->fp = fp;\n\ts->nowarn = nowarn;\n\ts->is_initial = get_oid(s->reference, &oid) ? 1 : 0;\n\tif (!s->is_initial)\n\t\thashcpy(s->sha1_commit, oid.hash);\n\ts->status_format = status_format;\n\ts->ignore_submodule_arg = ignore_submodule_arg;\n\n\twt_status_collect(s);\n\twt_status_print(s);\n\n\treturn s->commitable;\n}\n```\nWe can see the C version requires more lines of code and seem complex\nthan the script. But, this C code is not depending on external libraries\nand dependencies hence provides good performance and can be optimized\nfurther. The C codes after re-writing can be further documented,\noptimized and designed to make it more simpler for maintaining it. The\nfunctions can be documented with well-defined inputs, outputs and\nbehaviour. The internal git API can be used and the recurring codes can\nbe modified which can shrink the codebase and make git codebase smaller\nand resulting in smaller installation size in different platforms.\n\nNow, while doing this re-write there is high possibility of introducing\nbugs. Though, the test-suite can catch obvious bugs, other less-obvious\nbugs can escape from test scenarios. To fix this problem, new tests with\nthe co-ordination of other maintainers and contributors should be\nwritten for the cases where the test fails to catch bugs. Other code\ncoverage tools can also be used to ensure that the test suite for the\ncommand tests all code in builtin. Though it may take more time, careful\nrewriting and review of code must be done seriously by doing one to one\nline translations of shell scripts to C. The re-written code after tests\nmust go through rigorous review process to find and squash bugs present\nin it.\n\n> \"Given enough eyeballs, all bugs are shallow.\"\n>  - Linus Torvalds\n\nTimeline and Development Cycle\n------------------------------\n\n-   Apr 23: Accepted student proposals announced.\n-   Apr 23 onwards: Researching of all the test suites. Discussion of\n    possible test improvements in for `git-stash` and `git-rebase`.\n-   May 1: Rewriting skeleton begins.\n-   May 13: Making `builtin/stash.c` ready for review process. (This\n    goes on for some time.)\n-   May 26: Making `builtin/rebase.c` ready for review process. (This\n    goes on for some time.)\n-   June 10: Make second versions with more improvements and more\n    batteries ready for next review cycle.\n-   June 20: Writing new tests and using more code-coverage tools to\n    squash bugs present.\n-   June 25 - Jul 20: Start optimizing `builtin/stash.c` and\n    `builtin/rebase.c`. Benchmarking and profiling is done. They are\n    exclusively compared to their original shell scripts, to check\n    whether they are more performant or not and the results are\n    published in the mailing list for further discussion.\n-   Jul 20 - Aug 5: More optimizing and polishing of `builtin/stash.c`\n    and `builtin/rebase.c` and running of new tests series written and\n    send them for code review.\n-   Aug 14: Submit final patches.\n-   Aug 22: Results announced.\n-   Apr 24 - Aug 14: Documentation is written. \"What I'm working on\" is\n    written and posted in my blog regarding GSoC with Git.\n\nAbout me\n--------\n\nI'm Pratik Karki and am studing bachelors in Computer Engineering in\nAdvanced College of Engineering and Management (Affiliated to Institue\nof Engineering, Tribhuvan University). I've been writing C, C++, Ruby,\nPerl, Clojure, Lisp, Java, JS, Erlang, Rust for 3 years, and contributed\nto some projects which can be seen in\n[Github](https://github.com/prertik). I've been doing independent\ncontract works for small upcoming start-ups in Nepal.\nI have been planning to contribute in Git for a long time. Thanks, to\nGSoC I've submitted small patch as a microproject: [test: avoid pipes in\ngit related commands for test\nsuite](https://public-inbox.org/git/20180319173204.31952-1-predatoramigo@gmail.com/).\nI am looking forward to submitting more patches to Git on a long time\nbasis.\n"},{"id":"342321","messageId":"nycvar.QRO.7.76.6.1803201616290.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48094","inReplyTo":"20180320093421.26817-1-predatoramigo@gmail.com","subject":"Re: [RFC] [GSoC] Project proposal: convert scripts to builtins","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-20T15:28:22Z","receivedAt":"2018-03-20T15:28:42Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Pratik,\n\nthank you so much for posting this inline, to make it easier to review.\n\nI will quote only on specific parts below; Please just assume that I like\nthe other parts and have nothing to add.\n\nOn Tue, 20 Mar 2018, Pratik Karki wrote:\n\n> \n> Timeline and Development Cycle\n> ------------------------------\n> \n> -   Apr 23: Accepted student proposals announced.\n> -   Apr 23 onwards: Researching of all the test suites. Discussion of\n>     possible test improvements in for `git-stash` and `git-rebase`.\n> -   May 1: Rewriting skeleton begins.\n\nI would have liked more detail here. Like, maybe even a rudimentary\ninitial version identifying, say, a part of `git stash` and/or `git\nrebase` that could be put into a builtin (stash--helper and\nrebase--helper, respectively).\n\nIt is my experience from several GSoCs working on this huge overarching\nproject to convert the scripts (which are good prototypes, but lack in\nstringency in addition to performance) to C that even the individual\nscripts are too much to stem for a single GSoC.\n\n> -   May 13: Making `builtin/stash.c` ready for review process. (This\n>     goes on for some time.)\n\nThere have been two past efforts to turn stash into a builtin:\n\n\thttps://github.com/git-for-windows/git/pull/508\n\nand\n\n\thttps://public-inbox.org/git/20171110231314.30711-1-joel@teichroeb.net/\n\nIt would be good to read up on those and incorporate the learnings into\nthe proposal.\n\n> -   May 26: Making `builtin/rebase.c` ready for review process. (This\n>     goes on for some time.)\n\nThe `git-rebase.sh` script is itself not terribly interesting, as it hands\noff to `git-rebase--am.sh`, `git-rebase--interactive.sh` and\n`git-rebase--merge.sh`, respectively.\n\nConverting `git-rebase` into a builtin without first converting all of\nthose scripts would make little sense.\n\nIt would probably be better to choose one of those latter scripts and move\ntheir functionality into a builtin, in an incremental fashion.\n\nBy doing it incrementally, you can also avoid...\n\n> -   June 10: Make second versions with more improvements and more\n>     batteries ready for next review cycle.\n\n... leaving two weeks between checkpoints. Also, doing it incrementally\nlets you avoid sitting on your hands while waiting for the first patches\nto be reviewed.\n\n> -   June 20: Writing new tests and using more code-coverage tools to\n>     squash bugs present.\n\nTypically it helps a lot to have those tests *during* the conversion.\nThat's how I found most of the bugs when converting difftool, for example.\n\n> -   June 25 - Jul 20: Start optimizing `builtin/stash.c` and\n>     `builtin/rebase.c`. Benchmarking and profiling is done. They are\n>     exclusively compared to their original shell scripts, to check\n>     whether they are more performant or not and the results are\n>     published in the mailing list for further discussion.\n\nCould you add details how you would perform benchmarking and profiling?\n\n> -   Jul 20 - Aug 5: More optimizing and polishing of `builtin/stash.c`\n>     and `builtin/rebase.c` and running of new tests series written and\n>     send them for code review.\n> -   Aug 14: Submit final patches.\n> -   Aug 22: Results announced.\n> -   Apr 24 - Aug 14: Documentation is written. \"What I'm working on\" is\n>     written and posted in my blog regarding GSoC with Git.\n\nThe timeline is a bit ambitious. I would like to caution you that these\nare all big tasks, and maybe you want to cut down on the deliverables, and\nadd more detail what exactly you want to deliver (such as: what part of\nstash/rebase do you find under-tested in our test suite and would\ntherefore want to augment, what parts of stash/rebase do you think you\nwould handle first, and how?).\n\nCiao,\nJohannes\n"},{"id":"342386","messageId":"20180321061605.27814-1-predatoramigo@gmail.com","threadId":"48094","inReplyTo":"nycvar.QRO.7.76.6.1803201616290.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [RFC] [GSoC] Project proposal: convert scripts to builtins","fromName":"Pratik Karki","fromEmail":"predatoramigo@gmail.com","sentAt":"2018-03-21T06:16:05Z","receivedAt":"2018-03-21T06:16:36Z","isPatch":false,"sender":{"key":"predatoramigo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12194950?v=4"},"body":"Hi Johannes,\n\nThanks for the feedback. Thanks to you, I realized my proposal was\na bit ambitious. Both git-stash and git-rebase are big\ncommitment. After much analyzing, I found out I cannot complete\nboth in the given time frame. So, I decided to stick to one and\ncomplete it. I decided to stick with git-stash. Thank you for directing\nme to the un-merged matches. Now, I can find the points where the\npatch couldn't be effective and work towards completing those\neffective things.\n\nPlease provide feedback for this updated proposal.\n\nCheers,\nPratik Karki\n\n\nConvert Scripts to builtins\n===========================\n\nAbstract\n--------\n\nMany components of Git are still in the form of shell and Perl scripts.\nThis has certain advantages of being extensible but causes problems in\nproduction code on multiple platforms like Windows.\\\nI propose to rewrite a couple of shell and perl scripts into portable\nand performant C code, making them built-ins. The major advantage of\ndoing this is improvement in efficiency and performance.\n\nMuch more scripts like `git-am` , `git-pull`, `git-branch` have already\nbeen rewritten in C. Much more scripts like `git-rebase`, `git-stash`,\n`git-add --interactive` are still present in shell and perl scripts. I\npropose to work in `git-stash`.\n\n### Shell Scripts:\n\nAlthough shell scripts are more faster can be extensible in\nfunctionality and can be more easier to write, they introduce certain\ndisadvantages.\n\n1.  Dependencies:\\\n     The scripting languages and shell scripts make more productive code\n    but there is an overhead of dependencies. The shell scripts are\n    lighter and simpler and call other executables to perform\n    non-trivial tasks. Taking `git-stash` shell script for example.\n    `sed`, `rm`, `echo`, `test` are constantly present in `git-stash`.\n    These look common to POSIX platforms but for non-POSIX platforms\n    there needs some extra work for porting these commands. For example,\n    in Git for Windows, the workaround for these commands in non-POSIX\n    platform adds some extra utilities and adds MSYS2 shell commands and\n    needs to pack language runtime for Perl. This increases the\n    installation size and requires more disk space. Again, adding more\n    batteries again needs implementation in all of the dependency\n    libraries and executables.\n\n2.  Inefficiency:\\\n     Git has internal caches for configuration values, the repository\n    index and repository objects. The porcelain commands do not have\n    access to git's internal API and so they spawn git processes to\n    perform git operations. For every git invocation, git would re-read\n    the user's configuration files, repository index, repopulate the\n    filesystem cache, etc. This leads to overhead and unnecessary I/O.\n    Windows is known to have worse I/O performance compared to Linux.\n    There is also slower I/O performance of HDD compared to SSD. This\n    unnecessary I/O operations causes runtime overhead and becomes\n    slower in poor I/O performance setups. Now, writing the porcelain\n    into C built-ins leverages the git API and there is no need of\n    spawning separate git processes, caching can be used to reduce\n    unnecessary I/O processes.\n\n3.  Spawing processes is less performant:\\\n     Shell scripts usually spawn a lot of processes. Shell scripts are\n    very lighter and hence have limited functionalites. For\n    `git-stash.sh` to work it needs to perform lots of git operations\n    like `git rev-parse` `git config` and thus spawns git executable\n    processes for performing these operations. Again for invoking\n    `git config` and providing configuration values, it spawn new\n    processes to handle that. Spawning is implemented by `fork()` and\n    `exec()` by shells. Now, on systems that do not support\n    copy-on-write semantics for `fork()`, there is duplication of the\n    memory of the parent process for every `fork()` call which turns out\n    to be an expensive process. Now, in Windows, Git uses MSYS2\n    exclusively to emulate `fork()` but since, Windows doesnot support\n    forking semantics natively, the workaround provided by MSYS2\n    emulates `fork()` without [copy-on-write\n    semantics](https://www.cygwin.com/faq.html#faq.api.fork). Doing this\n    creates another layer over Windows processes and thus slows git.\n\nRewriting C built-ins\n---------------------\n\nThese above mentioned problems need to be fixed. The only fix for these\nproblems would be to write built-ins in C for all these shell scripts\nleveraging the git API. Writing in built-in reduces the dependency\nrequired by shell scripts. Since, Git is native executable in Windows,\ndoing this can make MSYS2 POSIX emulation obsolete. Then, using git's\ninternal API and C data types, built-in `git_config_get_value()` can be\nused to get configuration value rather than spawning another git-config\nprocess. This removes the necessary to re-read git configuration cache\neverytime and reduces I/O. Furthermore, git-stash will be more faster\nand show consistent behaviour as instead of spawing another process and\nparsing command-line arguments manually, they can be hardcoded to be\nbuilt-in and leverage all the required git's internal API's like\n`parse-options`.\n\nTo implement git-stash in C, I propose to avoid spawning lots of\nexternal git processes and reduce redundant I/O by taking advantage of\nthe internal object, index and configuration cache. I propose to use C\ndata structures instead of needless parsing as C has richer and\nperformant data structures. Windows will definitely benefit from these\nrewrites as the use of MSYS2 POSIX emulation would be obsolete and\nunnecessary I/O processes would be reduced and there won't be need of\nspawning external git processes.\n\nBut, while rewriting regressions should be totally avoided. The code\nshould not cause regression and introduce bugs not present in previous\nscripts. Thought the re-write wouldn't look more simpler like previous\nscripts, it should not have different behaviour or bugs compared to the\nscript and should not be hard to maintain compared to the script.\n\nThere has been some development in `git-stash` as seen on\n[<https://public-inbox.org/git/20171110231314.30711-1-joel@teichroeb.net/>]\n(https://public-inbox.org/git/20171110231314.30711-1-joel@teichroeb.net/).\nTo maximize the productivity, the findings from the patch submitted can\nbe used. Since, there are already much discussions regarding the\nrewrite.\n\nPotential Problems\n------------------\n\n### New Re-write code becomes less simpler and Bugs could get introduced:\n\nC is a more verbose language than shell script and looks less simpler\nthan shell scripts. Doing things in less than 4 lines of shell script in\nC can take up more than 12 lines to make it more simpler. For example,\nin a previously used script `git-commit.sh` we can see,\n\n```bash\n     run_status () {\n\t# If TMP_INDEX is defined, that means we are doing\n\t# \"--only\" partial commit, and that index file is used\n\t# to build the tree for the commit.  Otherwise, if\n\t# NEXT_INDEX exists, that is the index file used to\n\t# make the commit.  Otherwise we are using as-is commit\n\t# so the regular index file is what we use to compare.\n\tif test '' != \"$TMP_INDEX\"\n\tthen\n\t\tGIT_INDEX_FILE=\"$TMP_INDEX\"\n\t\texport GIT_INDEX_FILE\n\telif test -f \"$NEXT_INDEX\"\n\tthen\n\t\tGIT_INDEX_FILE=\"$NEXT_INDEX\"\n\t\texport GIT_INDEX_FILE\n\tfi\n\n\tif test \"$status_only\" = \"t\" -o \"$use_status_color\" = \"t\"; then\n\t\tcolor=\n\telse\n\t\tcolor=--nocolor\n\tfi\n\tgit runstatus ${color} \\\n\t\t${verbose:+--verbose} \\\n\t\t${amend:+--amend} \\\n\t\t${untracked_files:+--untracked}\n    }\n ```\nThis script has been rewritten as builtin in C. These lines of code\nhave been replaced in C providing more built-in performance\nbut looks less simpler and contains high multiline functions in `commit.c`\n\n```C\n`static int run_status(FILE *fp, const char *index_file, const char *prefix, int nowarn,\n\t\t      struct wt_status *s)\n{\n\tstruct object_id oid;\n\n\tif (s->relative_paths)\n\t\ts->prefix = prefix;\n\n\tif (amend) {\n\t\ts->amend = 1;\n\t\ts->reference = \"HEAD^1\";\n\t}\n\ts->verbose = verbose;\n\ts->index_file = index_file;\n\ts->fp = fp;\n\ts->nowarn = nowarn;\n\ts->is_initial = get_oid(s->reference, &oid) ? 1 : 0;\n\tif (!s->is_initial)\n\t\thashcpy(s->sha1_commit, oid.hash);\n\ts->status_format = status_format;\n\ts->ignore_submodule_arg = ignore_submodule_arg;\n\n\twt_status_collect(s);\n\twt_status_print(s);\n\n\treturn s->commitable;\n}\n```\n\nWe can see the C version requires more lines of code and seem complex\nthan the script. But, this C code is not depending on external libraries\nand dependencies hence provides good performance and can be optimized\nfurther. The C codes after re-writing can be further documented,\noptimized and designed to make it more simpler for maintaining it. The\nfunctions can be documented with well-defined inputs, outputs and\nbehaviour. The internal git API can be used and the recurring codes can\nbe modified which can shrink the codebase and make git codebase smaller\nand resulting in smaller installation size in different platforms.\n\nNow, while doing this re-write there is high possibility of introducing\nbugs. Though, the test-suite can catch obvious bugs, other less-obvious\nbugs can escape from test scenarios. To fix this problem, new tests with\nthe co-ordination of other maintainers and contributors should be\nwritten for the cases where the test fails to catch bugs. Other code\ncoverage tools can also be used to ensure that the test suite for the\ncommand tests all code in builtin. Though it may take more time, careful\nrewriting and review of code must be done seriously by doing one to one\nline translations of shell scripts to C. The re-written code after tests\nmust go through rigorous review process to find and squash bugs present\nin it.\n\n> \"Given enough eyeballs, all bugs are shallow.\"\n>  - Linus Torvalds\n\nTimeline and Development Cycle\n------------------------------\n\n-   Apr 23: Accepted student proposals announced.\n\n-   Apr 23 onwards: Researching of all the test suites. Discussion of\n    possible test improvements in for `git-stash`.\n\n    Firstly, the test suite coverage of every command will be reviewed\n    using gcov and kcov.The test suite might not be perfect or\n    comprehensive but must cover all the major code paths and\n    command-line switches of the script. For the tests which seem\n    inadequate, minimum required tests are written and developed\n    incrementally. The minimum tests must provide safety net for\n    migration of scripts to built-ins. The tests would be sent as a\n    separate patch for parallel development and review process so that\n    development of built-ins can happen at the same time productively.\n    The tests will be written for every code changes and will be worked\n    throughout the summer.\n\n-   May 1: Rewriting skeleton begins.\n\n    The shell scripts are translated on a line-by-line basis into C\n    code. The C code will be written in a way to maximize the use of git\n    internal API. In git-stash `parse-options` API can be used for\n    implementing parsing argument of command-line. This would be way\n    better than parsing via the scripts. Firstly, I will start\n    implementing `stash --helper`from respective scripts to C code. Then\n    increment it further more. Then I'll start converting git-stash.sh\n    on a line-by-line basis.\\\n     Again for git-stash some work seem to be done\n    [<https://public-inbox.org/git/20171110231314.30711-1-joel@teichroeb.net/>]\n    (https://public-inbox.org/git/20171110231314.30711-1-joel@teichroeb.net/).\n    Now, to maximize the output I'll be taking findings from the\n    previous patch and use it for my patch. As seen from the comments in\n    the patch some tests for checking branch when `git stash branch`\n    fails needs to be written. New tests will be written and code\n    coverage tools will be used for the written code.\n\n-   May 13: Making minimal `builtin/stash.c` with `stash--helper` ready\n    for review process. (This goes on for some time.)\n\n    The initial review of minimal builtin would be ready for git-stash.\n    The result C code at this stage may not be necessarily be efficient\n    but would be free from obvious bugs and can serve as a baseline for\n    the final patch. This is sent for review process which can take some\n    time. The code will ofcourse be tested using the test suite with\n    some additional tests.\n\n-   May 13 - June 10: Make second versions with more improvements and\n    more batteries ready for next review cycle.\n\n    The improved versions after rigorous review process is made ready\n    for `builtin/stash.c`. This improved version will be functionally\n    equivalent to the previous scripts and will be error free and will\n    be ready for optimization and will be very extensible. It will use\n    the internal API and can be optimized for faster processes. Certain\n    recurring codes at this point are modified to decrease the code\n    base.\\\n    At this point, the previous written tests will need some development\n    and with the more understanding of the working of the builtin code,\n    the tests are again optimized and code-coverage tools like gcov is\n    used. The tests are again updated and sent over in a separate patch.\n    The tests written at this stage will be very efficient and cover\n    across all code bases effectively.\n\n-   June 10 - Jul 20: Start optimizing `builtin/stash.c`. Benchmarking\n    and profiling is done. They are exclusively compared to their\n    original shell scripts, to check whether they are more performant or\n    not and the results are published in the mailing list for further\n    discussion.\n\nThe C code will be optimized for speed and efficiency in this stage. The\nbuilt-ins will now be profiled using the new efficient test suites to\nfind hot spots. Bench-marking is also done in comparison to original\nscripts.The performance for stash can be measured by making it stash\nlarge number of changes in another working directory and measuring the\ntime for completion of the task. After finding out, a graphical\nrepresentation of performance findings will be published to git mailing\nlist and discussions will commence on more optimization.\n\n-   Jul 20 - Aug 5: More optimizing and polishing of `builtin/stash.c`\n    and further polishing of tests series written and send them for code\n    review.\n\nAfter discussions with the git community, optimization is done further\nand the code and tests at this stage are polished for final submissions.\nThe tests are polished and rigorous testing of bugs and performance\npenalty is done. This will help to find previous hidden errors if\npresent.\n\n-   Aug 14: Submit final patches.\n     The final patch series containing all the summer's code is\n    submitted for final review and merge to git.\n\n-   Aug 22: Results announced.\n\n-   Apr 24 - Aug 14: Documentation is written. \"What I'm working on\" is\n    written and posted in my blog regarding GSoC with Git.\n\n    The documentation of the code written is done in the whole summer.\n    Additionally, a blog series will be written in a weekly basis of my\n    current findings and will write about \"What I'm working on\" to\n    further provide information about my development of summer project.\n\nAbout me\n--------\n\nI'm Pratik Karki and am studing bachelors in Computer Engineering in\nAdvanced College of Engineering and Management (Affiliated to Institue\nof Engineering, Tribhuvan University). I've been writing C, C++, Ruby,\nPerl, Clojure, Lisp, Java, JS, Erlang, Rust for 3 years, and contributed\nto some projects which can be seen in\n[Github](https://github.com/prertik). I've been doing independent\ncontract works for small upcoming start-ups in Nepal.\nI have been planning to contribute in Git for a long time. Thanks, to\nGSoC I've submitted small patch as a microproject: [test: avoid pipes in\ngit related commands for test\nsuite](https://public-inbox.org/git/20180319173204.31952-1-predatoramigo@gmail.com/).\nI am looking forward to submitting more patches to Git on a long time\nbasis.\n"},{"id":"342812","messageId":"CAP8UFD2k6ROC51zHob+HnqqKL0fgM=xLgUk3LmOJTTEuoHBijQ@mail.gmail.com","threadId":"48094","inReplyTo":"20180321061605.27814-1-predatoramigo@gmail.com","subject":"Re: [RFC] [GSoC] Project proposal: convert scripts to builtins","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-03-24T10:39:03Z","receivedAt":"2018-03-24T10:39:13Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nOn Wed, Mar 21, 2018 at 7:16 AM, Pratik Karki <predatoramigo@gmail.com> wrote:\n>\n> Thanks for the feedback. Thanks to you, I realized my proposal was\n> a bit ambitious. Both git-stash and git-rebase are big\n> commitment. After much analyzing, I found out I cannot complete\n> both in the given time frame. So, I decided to stick to one and\n> complete it.\n\nGreat.\n\n[...]\n\n> There has been some development in `git-stash` as seen on\n> [<https://public-inbox.org/git/20171110231314.30711-1-joel@teichroeb.net/>]\n> (https://public-inbox.org/git/20171110231314.30711-1-joel@teichroeb.net/).\n> To maximize the productivity, the findings from the patch submitted can\n> be used. Since, there are already much discussions regarding the\n> rewrite.\n\nIn general it would be nice if you summarized what has already been\ndone, how you can reuse it and what is needed to complete it.\n\nI see that you talk about some of that below, but a more general\noverview might be nice too.\n\nIt could be interesting also to put the author(s) of the work that you\nwill reuse in Cc.\n\n[...]\n\n> Timeline and Development Cycle\n> ------------------------------\n>\n> -   Apr 23: Accepted student proposals announced.\n>\n> -   Apr 23 onwards: Researching of all the test suites. Discussion of\n>     possible test improvements in for `git-stash`.\n>\n>     Firstly, the test suite coverage of every command will be reviewed\n>     using gcov and kcov.\n\nI don't think it is necessary to spend a lot of time on the test suite coverage.\n\n> The test suite might not be perfect or\n>     comprehensive but must cover all the major code paths and\n>     command-line switches of the script. For the tests which seem\n>     inadequate, minimum required tests are written and developed\n>     incrementally. The minimum tests must provide safety net for\n>     migration of scripts to built-ins. The tests would be sent as a\n>     separate patch for parallel development and review process so that\n>     development of built-ins can happen at the same time productively.\n\nNice.\n\n>     The tests will be written for every code changes and will be worked\n>     throughout the summer.\n>\n> -   May 1: Rewriting skeleton begins.\n>\n>     The shell scripts are translated on a line-by-line basis into C\n>     code. The C code will be written in a way to maximize the use of git\n>     internal API. In git-stash `parse-options` API can be used for\n>     implementing parsing argument of command-line. This would be way\n>     better than parsing via the scripts. Firstly, I will start\n>     implementing `stash --helper`from respective scripts to C code. Then\n>     increment it further more. Then I'll start converting git-stash.sh\n>     on a line-by-line basis.\n\nNot sure what you mean by line-by-line basis.\n\n>      Again for git-stash some work seem to be done\n>     [<https://public-inbox.org/git/20171110231314.30711-1-joel@teichroeb.net/>]\n>     (https://public-inbox.org/git/20171110231314.30711-1-joel@teichroeb.net/).\n>     Now, to maximize the output I'll be taking findings from the\n>     previous patch and use it for my patch. As seen from the comments in\n>     the patch some tests for checking branch when `git stash branch`\n>     fails needs to be written.\n\nNice. Maybe writing those tests can come earlier in you schedule.\n\n> New tests will be written and code\n>     coverage tools will be used for the written code.\n\nNot sure that code coverage tools need to be used.\n\n> -   May 13: Making minimal `builtin/stash.c` with `stash--helper` ready\n>     for review process. (This goes on for some time.)\n>\n>     The initial review of minimal builtin would be ready for git-stash.\n>     The result C code at this stage may not be necessarily be efficient\n>     but would be free from obvious bugs and can serve as a baseline for\n>     the final patch. This is sent for review process which can take some\n>     time. The code will ofcourse be tested using the test suite with\n>     some additional tests.\n\nHow does that relates with the existing work? Will this be one or\nseveral patch series? What will each patch do?\n\n[...]\n\n> -   June 10 - Jul 20: Start optimizing `builtin/stash.c`. Benchmarking\n>     and profiling is done. They are exclusively compared to their\n>     original shell scripts, to check whether they are more performant or\n>     not and the results are published in the mailing list for further\n>     discussion.\n\nWill the performance tests be added to the t/perf tests?\n\n> The C code will be optimized for speed and efficiency in this stage. The\n> built-ins will now be profiled using the new efficient test suites to\n> find hot spots. Bench-marking is also done in comparison to original\n> scripts.The performance for stash can be measured by making it stash\n> large number of changes in another working directory and measuring the\n> time for completion of the task. After finding out, a graphical\n> representation of performance findings will be published to git mailing\n> list and discussions will commence on more optimization.\n>\n> -   Jul 20 - Aug 5: More optimizing and polishing of `builtin/stash.c`\n>     and further polishing of tests series written and send them for code\n>     review.\n>\n> After discussions with the git community, optimization is done further\n> and the code and tests at this stage are polished for final submissions.\n\nOften there is not much time for optimization and a lot more time is\nspent on improving the structure of the patch series and the patch\nthemselves through many review rounds. We also expect that many of the\nearly patch series have been already polished and submitted many times\nbefore the last weeks of the GSoC.\n\n[...]\n\n> -   Apr 24 - Aug 14: Documentation is written.\n\nWhat kind of documentation is that? In the Git project we expect that\nthe documentation for the work done comes with the work itself. It can\nbe in the form of the cover letter of a patch series, or the commit\nmessages of the patches that are sent to the mailing list, or in\npatches that change files in Documentation/.\n\n> \"What I'm working on\" is\n>     written and posted in my blog regarding GSoC with Git.\n>\n>     The documentation of the code written is done in the whole summer.\n>     Additionally, a blog series will be written in a weekly basis of my\n>     current findings and will write about \"What I'm working on\" to\n>     further provide information about my development of summer project.\n\nNice.\n\n> About me\n> --------\n>\n> I'm Pratik Karki and am studing bachelors in Computer Engineering in\n> Advanced College of Engineering and Management (Affiliated to Institue\n> of Engineering, Tribhuvan University). I've been writing C, C++, Ruby,\n> Perl, Clojure, Lisp, Java, JS, Erlang, Rust for 3 years, and contributed\n> to some projects which can be seen in\n> [Github](https://github.com/prertik). I've been doing independent\n> contract works for small upcoming start-ups in Nepal.\n> I have been planning to contribute in Git for a long time. Thanks, to\n> GSoC I've submitted small patch as a microproject: [test: avoid pipes in\n> git related commands for test\n> suite](https://public-inbox.org/git/20180319173204.31952-1-predatoramigo@gmail.com/).\n> I am looking forward to submitting more patches to Git on a long time\n> basis.\n\nThanks.\n"}]}