{"thread":{"id":"50863","subject":"[GSoC][RFC] proposal: convert git-submodule to builtin script","startedAt":"2019-04-02T20:32:41Z","lastAt":"2019-04-04T08:30:52Z","messageCount":4,"participants":["Khalid Ali","Johannes Schindelin","Christian Couder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"372994","messageId":"CAD8q83SifH-7mKOet-Uuf_KHCqvO2mkxiZgEyQhoNRhshFG_Lg@mail.gmail.com","threadId":"50863","inReplyTo":null,"subject":"[GSoC][RFC] proposal: convert git-submodule to builtin script","fromName":"Khalid Ali","fromEmail":"khalludi123@gmail.com","sentAt":"2019-04-02T20:32:26Z","receivedAt":"2019-04-02T20:32:41Z","isPatch":false,"sender":{"key":"khalludi123@gmail.com","avatar":null},"body":"Hi,\n\nMy name is Khalid Ali and I am looking to convert the git-submodule to\na builtin C script. The link below contains my first proposal draft\n[1] and my microproject is at [2]. My main concern is that my second\ntask is not verbose enough. I am not sure if I should add a specific\nbreakdown of large items within the submodule command.\n\nOutside of the draft, I was wondering whether this should be\nimplemented through multiple patches to the master branch or through a\nseparate, long-running feature branch that will be merged at the end\nof the GSoC timeline?\n\nFeedback is greatly appreciated!\n\n[1] https://docs.google.com/document/d/1olGG8eJxFoMNyGt-4uMiTD3LjRYx15pttg67AJYliu8/edit?usp=sharing\n[2] https://public-inbox.org/git/20190402014115.22478-1-khalludi123@gmail.com/\n"},{"id":"373065","messageId":"nycvar.QRO.7.76.6.1904031957480.41@tvgsbejvaqbjf.bet","threadId":"50863","inReplyTo":"CAD8q83SifH-7mKOet-Uuf_KHCqvO2mkxiZgEyQhoNRhshFG_Lg@mail.gmail.com","subject":"Re: [GSoC][RFC] proposal: convert git-submodule to builtin script","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-04-03T18:48:28Z","receivedAt":"2019-04-03T18:48:49Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 2 Apr 2019, Khalid Ali wrote:\n\n> My name is Khalid Ali and I am looking to convert the git-submodule to\n> a builtin C script. The link below contains my first proposal draft\n> [1] and my microproject is at [2]. My main concern is that my second\n> task is not verbose enough. I am not sure if I should add a specific\n> breakdown of large items within the submodule command.\n\nNice!\n\nPlease note that while I used to be the mentor who basically helped all of\nthe GSoC/Outreachy students through their \"convert to built-in\" projects\nin the recent years, I am not available to mentor this year.\n\nHaving said that, I think I can help you to improve your proposal.\n\nWhen you talk about \"Convert each main task in git-submodule into a C\nfunction.\" and \"If certain functionality is missing, add it to the correct\nscript.\", it is a good idea to back that up by concrete examples.\n\nLike, study `git-submodule.sh` and extract the list of \"main tasks\", and\nthen mention that in your proposal. I see that you listed 9 main tasks,\nbut it is not immediately clear whether you extracted that list from the\nusage text, from the manual page, or from the script itself. If the latter\n(which I think would be the best, given the goal of converting the code in\nthat script), it would make a ton of sense to mention the function names\nand maybe add a permalink to the corresponding code (you could use e.g.\nGitHub's permalinks).\n\nAnd then look at one of those main tasks, think of something that you\nbelieve should be covered in the test suite, describe it, then figure out\nwhether it is already covered. If it is, mention that, together with the\nlocation, otherwise state which script would be the best location, and\nwhy.\n\nFurther, I would like to caution you about \"If there is still some\ntime\"... The `git-submodule.sh` script weighs in with just over 1,000\nlines. We had three GSoC projects to convert scripts last year, and they\nconverted scripts' weights (at the time) were 750 lines for\n`git-stash.sh`, 674 lines for `git-rebase.sh` and 1,036 lines for\n`git-rebase--interactive.sh`, respectively. That last number should be\ntaken with a big grain of salt, as is not quite the number of lines that\nwere converted: as part of the GSoC project, the\n`git-rebase--preserve-merges.sh` script was split out, never intended to\nbe converted, but to be deprecated instead (in favor of `git rebase -r`),\nand there were \"only\" some 283 lines to be converted to C remaining after\nthat.\n\nOut of those three, the project converting the smallest number of lines\nclearly got integrated first (and there was actually time to do more stuff\nin that project, and those things are partially still being cooked). The\nconverted `git stash` is still not in `master`...\n\nSo... converting 1,000 lines of code is quite a challenge for 3 months.\n\nHaving said that, I would not consider your project a failure if even\n\"only\" as much as half of the lines of code were converted to C.\n\nBesides, if you care to have a bit of a deeper look into the\n`git-submodule.sh` script, you will see a peculiar pattern in some of the\nsubcommands, e.g. in `cmd_foreach`:\nhttps://github.com/git/git/blob/v2.21.0/git-submodule.sh#L320-L349\n\nEssentially, it spends two handfuls of lines on option parsing, and then\nthe real business logic is performed by the `submodule--helper`, which is\n*already* a built-in.\n\nEven better: most of that business logic is implemented in a file that has\nthe very file name you proposed already: `submodule.c`.\n\nSo if I were you, I would add a section to your proposal (which in the end\nwould no doubt dwarf the existing sections) that has as subsections each\nof those commands in `git-submodule.sh` that do *not* yet follow this\npattern \"parse options then hand off to submodule--helper\".\n\nI would then study the commit history of the ones that *do* use the\n`submodule--helper` to see how they were converted, what conventions were\nused, whether there were recurring patterns, etc.\n\nIn each of those subsections, I would then discuss what the\nstill-to-be-converted commands do, try to find the closest command that\nalready uses the `submodule--helper`, and then assess what it would take\nto convert them, how much code it would probably need, whether it could\nreuse parts that are already in `submodule.c`, etc.\n\n> Outside of the draft, I was wondering whether this should be\n> implemented through multiple patches to the master branch or through a\n> separate, long-running feature branch that will be merged at the end\n> of the GSoC timeline?\n\nJudging from past projects to convert scripts to C, I would say that the\nmost successful strategy was to chomp off manageable parts and move them\nfrom the script to C. I am sure that you will find tons of good examples\nfor this strategy by looking at the commit history of `git-submodule.sh`\nand then searching for the corresponding patches in the Git mailing list\narchive (e.g. https://public-inbox.org/git/).\n\nDo not expect those \"chomped off\" parts to hit `master` very quickly,\nthough. Most likely, you would work on one patch series (very closely with\nyour mentor at first, to avoid unnecessary blocks and to get a better feel\nfor the way the Git community works right from the start), then, when that\npatch series is robust and solid and ready to be contributed, you would\nsend it to the Git mailing list and immediately start working on the next\npatch series, all the while the reviews will trickle in. Those reviews\nwill help you to improve the patch series, and it is a good idea to\nincorporate the good suggestions, and to discuss the ones you think are\nnot necessary, for a few days before sending the next patch series\niteration.\n\nEssentially, you will work in parallel on a few patch series at all times.\nThose patch series stack on top of each other, and they should, one after\nthe other, make it into `pu` first, then, when they are considered ready\nfor testing into `next`, and eventually to `master`. Whenever you\ncontribute a new patch series iteration, you then rebase the remaining\npatch series on top. Ideally it will look a bit like a fern, with the\nfirst patch series being along the farthest, and each subsequent patch\nseries at an earlier stage than its predecessor.\n\nPhew. Long mail. Hopefully this amount of information does not scare you.\nAnd maybe some of it will help you with the proposal and/or the project.\n\nCiao,\nJohannes\n\n> Feedback is greatly appreciated!\n>\n> [1] https://docs.google.com/document/d/1olGG8eJxFoMNyGt-4uMiTD3LjRYx15pttg67AJYliu8/edit?usp=sharing\n> [2] https://public-inbox.org/git/20190402014115.22478-1-khalludi123@gmail.com/\n>\n"},{"id":"373070","messageId":"CAD8q83Rz6myozY1aJqaZjEfjgmSkQ0059=m=mmkJ9O90u+9spw@mail.gmail.com","threadId":"50863","inReplyTo":"nycvar.QRO.7.76.6.1904031957480.41@tvgsbejvaqbjf.bet","subject":"Re: [GSoC][RFC] proposal: convert git-submodule to builtin script","fromName":"Khalid Ali","fromEmail":"khalludi123@gmail.com","sentAt":"2019-04-03T21:07:34Z","receivedAt":"2019-04-03T21:07:52Z","isPatch":false,"sender":{"key":"khalludi123@gmail.com","avatar":null},"body":"First of all, thank you so much for the detailed feedback. I wasn't sure\nhow much to include in the proposal, but I see it still needs a lot of work.\n\n> When you talk about \"Convert each main task in git-submodule into a C\n> function.\" and \"If certain functionality is missing, add it to the correct\n> script.\", it is a good idea to back that up by concrete examples.\n>\n> Like, study `git-submodule.sh` and extract the list of \"main tasks\", and\n> then mention that in your proposal. I see that you listed 9 main tasks,\n> but it is not immediately clear whether you extracted that list from the\n> usage text, from the manual page, or from the script itself. If the latter\n> (which I think would be the best, given the goal of converting the code in\n> that script), it would make a ton of sense to mention the function names\n> and maybe add a permalink to the corresponding code (you could use e.g.\n> GitHub's permalinks).\n\nYes, I actually did extract the tasks straight from git-submodule.sh. I will\ndefinitely add the appropriate function names and permalinks to the\nproposal.\n\n> And then look at one of those main tasks, think of something that you\n> believe should be covered in the test suite, describe it, then figure out\n> whether it is already covered. If it is, mention that, together with the\n> location, otherwise state which script would be the best location, and\n> why.\n\nAh, alright. I'll have a look at the test suite to see what is covered and\ninclude a section in my proposal.\n\n> Besides, if you care to have a bit of a deeper look into the\n> `git-submodule.sh` script, you will see a peculiar pattern in some of the\n> subcommands, e.g. in `cmd_foreach`:\n> https://github.com/git/git/blob/v2.21.0/git-submodule.sh#L320-L349\n>\n> Essentially, it spends two handfuls of lines on option parsing, and then\n> the real business logic is performed by the `submodule--helper`, which is\n> *already* a built-in.\n>\n> Even better: most of that business logic is implemented in a file that has\n> the very file name you proposed already: `submodule.c`.\n>\n> So if I were you, I would add a section to your proposal (which in the end\n> would no doubt dwarf the existing sections) that has as subsections each\n> of those commands in `git-submodule.sh` that do *not* yet follow this\n> pattern \"parse options then hand off to submodule--helper\".\n>\n> I would then study the commit history of the ones that *do* use the\n> `submodule--helper` to see how they were converted, what conventions were\n> used, whether there were recurring patterns, etc.\n>\n> In each of those subsections, I would then discuss what the\n> still-to-be-converted commands do, try to find the closest command that\n> already uses the `submodule--helper`, and then assess what it would take\n> to convert them, how much code it would probably need, whether it could\n> reuse parts that are already in `submodule.c`, etc.\n\nI definitely noticed the option parsing in multiple parts of the function, but\nthe pattern didn't click until you mentioned it. I'll do as you recommended\nand take a look at submodule.c to see how the code and functionality in\ngit-submodule.sh can be merged.\n\n> Judging from past projects to convert scripts to C, I would say that the\n> most successful strategy was to chomp off manageable parts and move them\n> from the script to C. I am sure that you will find tons of good examples\n> for this strategy by looking at the commit history of `git-submodule.sh`\n> and then searching for the corresponding patches in the Git mailing list\n> archive (e.g. https://public-inbox.org/git/).\n>\n> Do not expect those \"chomped off\" parts to hit `master` very quickly,\n> though. Most likely, you would work on one patch series (very closely with\n> your mentor at first, to avoid unnecessary blocks and to get a better feel\n> for the way the Git community works right from the start), then, when that\n> patch series is robust and solid and ready to be contributed, you would\n> send it to the Git mailing list and immediately start working on the next\n> patch series, all the while the reviews will trickle in. Those reviews\n> will help you to improve the patch series, and it is a good idea to\n> incorporate the good suggestions, and to discuss the ones you think are\n> not necessary, for a few days before sending the next patch series\n> iteration.\n>\n> Essentially, you will work in parallel on a few patch series at all times.\n> Those patch series stack on top of each other, and they should, one after\n> the other, make it into `pu` first, then, when they are considered ready\n> for testing into `next`, and eventually to `master`. Whenever you\n> contribute a new patch series iteration, you then rebase the remaining\n> patch series on top. Ideally it will look a bit like a fern, with the\n> first patch series being along the farthest, and each subsequent patch\n> series at an earlier stage than its predecessor.\n\nOk, so I'd be doing each of the portions and submitting them to the mailing\nlist as I finish to let other coders take a look and give feedback.\n\n> Phew. Long mail. Hopefully this amount of information does not scare you.\n> And maybe some of it will help you with the proposal and/or the project.\n\nOnce again, thank you for the detailed feedback. This really gave me a good\nidea of the project as a whole and what I need to include in my proposal.\n\nSincerely,\n\n-Khalid\n"},{"id":"373101","messageId":"CAP8UFD2z2au=Ftx828++kziwcEV+FaRrVJd4rqMw-N3=w5DN=w@mail.gmail.com","threadId":"50863","inReplyTo":"CAD8q83SifH-7mKOet-Uuf_KHCqvO2mkxiZgEyQhoNRhshFG_Lg@mail.gmail.com","subject":"Re: [GSoC][RFC] proposal: convert git-submodule to builtin script","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-04-04T08:30:38Z","receivedAt":"2019-04-04T08:30:52Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nOn Tue, Apr 2, 2019 at 10:34 PM Khalid Ali <khalludi123@gmail.com> wrote:\n>\n> My name is Khalid Ali and I am looking to convert the git-submodule to\n> a builtin C script. The link below contains my first proposal draft\n> [1] and my microproject is at [2]. My main concern is that my second\n> task is not verbose enough. I am not sure if I should add a specific\n> breakdown of large items within the submodule command.\n\nThere was a GSoC project about the same subject a few years ago:\n\nhttps://public-inbox.org/git/CAME+mvXtA6iZNfErTX5tYB-o-5xa1yesAG5h=iP_Z2_zL_kOnQ@mail.gmail.com/\n\nI think you should take a look at the work that was done (merged and\nnot merged) and report about it in your proposal.\n\nThanks,\nChristian.\n"}]}