{"thread":{"id":"59930","subject":"Documentation/MyFirstObjectWalk: add #include \"trace.h\" to use trace_printf()","startedAt":"2023-06-29T16:06:11Z","lastAt":"2023-06-30T15:40:09Z","messageCount":10,"participants":["Vinayak Dev","Emily Shaffer","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"479015","messageId":"CADE8Naq5W3Bn=gwV7W-xMvYOMMRO=ZY9Ly6im4Rb_qFjMWTbTg@mail.gmail.com","threadId":"59930","inReplyTo":null,"subject":"Documentation/MyFirstObjectWalk: add #include \"trace.h\" to use trace_printf()","fromName":"Vinayak Dev","fromEmail":"vinayakdev.sci@gmail.com","sentAt":"2023-06-29T16:05:51Z","receivedAt":"2023-06-29T16:06:11Z","isPatch":false,"sender":{"key":"vinayakdev.sci@gmail.com","avatar":"https://avatars.githubusercontent.com/u/104419489?v=4"},"body":"Hey there!\nI was looking through Documentation/MyFirstObjectWalk.txt, and upon\nbuilding the branch containing the given code, I find that I get the\nerror that C99 does not allow implicit function declaration where\ntrace_printf() is encountered. However, upon including trace.h the\nerror disappears, and the build proceeds just fine.\n\nI did put DEVELOPER=1 in config.mak before building, but it doesn't\nseem to work.\n\nIs the error pointing to a problem, or am I doing something wrong?\nIf it is the former, I would be very happy to send a patch fixing this.\n\nThanks a lot!\nVinayak\n"},{"id":"479016","messageId":"CAJoAoZ=OEfsgkqsag926tH4GEuafX26A09SGZ1vR1uLh2W_4TA@mail.gmail.com","threadId":"59930","inReplyTo":"CADE8Naq5W3Bn=gwV7W-xMvYOMMRO=ZY9Ly6im4Rb_qFjMWTbTg@mail.gmail.com","subject":"Re: Documentation/MyFirstObjectWalk: add #include \"trace.h\" to use trace_printf()","fromName":"Emily Shaffer","fromEmail":"nasamuffin@google.com","sentAt":"2023-06-29T16:33:49Z","receivedAt":"2023-06-29T16:35:03Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Jun 29, 2023 at 9:06 AM Vinayak Dev wrote:\n>\n> Hey there!\n> I was looking through Documentation/MyFirstObjectWalk.txt, and upon\n> building the branch containing the given code, I find that I get the\n> error that C99 does not allow implicit function declaration where\n> trace_printf() is encountered. However, upon including trace.h the\n> error disappears, and the build proceeds just fine.\n>\n> I did put DEVELOPER=1 in config.mak before building, but it doesn't\n> seem to work.\n>\n> Is the error pointing to a problem, or am I doing something wrong?\n> If it is the former, I would be very happy to send a patch fixing this.\n\nYeah, it's almost certainly stale in MyFirstObjectWalk - there was\nvery recently a patch to clean up some headers which probably were\nimplicitly including trace.h when I wrote this walkthrough. Patches\ntotally welcome - and if you were working from the reference code in\nhttps://github.com/nasamuffin/git/tree/myfirstrevwalk and it's on your\nway to rebase and fix that too, I'm happy to update my branch\naccordingly too. (If you weren't, don't worry about doing the extra\nwork, though.)\n\n>\n> Thanks a lot!\n> Vinayak\n"},{"id":"479017","messageId":"CAJoAoZnVAe3kvUdPZmanbKffG7cx3Tc-==H4+FH=L5qQP2smEg@mail.gmail.com","threadId":"59930","inReplyTo":"CAJoAoZ=OEfsgkqsag926tH4GEuafX26A09SGZ1vR1uLh2W_4TA@mail.gmail.com","subject":"Re: Documentation/MyFirstObjectWalk: add #include \"trace.h\" to use trace_printf()","fromName":"Emily Shaffer","fromEmail":"nasamuffin@google.com","sentAt":"2023-06-29T16:35:29Z","receivedAt":"2023-06-29T16:35:49Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Jun 29, 2023 at 9:33 AM Emily Shaffer wrote:\n>\n> On Thu, Jun 29, 2023 at 9:06 AM Vinayak Dev wrote:\n> >\n> > Hey there!\n> > I was looking through Documentation/MyFirstObjectWalk.txt, and upon\n> > building the branch containing the given code, I find that I get the\n> > error that C99 does not allow implicit function declaration where\n> > trace_printf() is encountered. However, upon including trace.h the\n> > error disappears, and the build proceeds just fine.\n> >\n> > I did put DEVELOPER=1 in config.mak before building, but it doesn't\n> > seem to work.\n> >\n> > Is the error pointing to a problem, or am I doing something wrong?\n> > If it is the former, I would be very happy to send a patch fixing this.\n>\n> Yeah, it's almost certainly stale in MyFirstObjectWalk - there was\n> very recently a patch to clean up some headers which probably were\n> implicitly including trace.h when I wrote this walkthrough. Patches\n> totally welcome - and if you were working from the reference code in\n> https://github.com/nasamuffin/git/tree/myfirstrevwalk\n\nbah, wrong link, the tutorial points to branch `revwalk` instead of\n`myfirstrevwalk`, but the offer stands :)\n\n> and it's on your\n> way to rebase and fix that too, I'm happy to update my branch\n> accordingly too. (If you weren't, don't worry about doing the extra\n> work, though.)\n>\n> >\n> > Thanks a lot!\n> > Vinayak\n"},{"id":"479021","messageId":"CADE8Naonm+bW_jVvJKmnfZWQyX=0-QVSHxpSaHs1qo+5DsCiPQ@mail.gmail.com","threadId":"59930","inReplyTo":"CAJoAoZnVAe3kvUdPZmanbKffG7cx3Tc-==H4+FH=L5qQP2smEg@mail.gmail.com","subject":"Re: Documentation/MyFirstObjectWalk: add #include \"trace.h\" to use trace_printf()","fromName":"Vinayak Dev","fromEmail":"vinayakdev.sci@gmail.com","sentAt":"2023-06-29T18:45:45Z","receivedAt":"2023-06-29T18:46:03Z","isPatch":false,"sender":{"key":"vinayakdev.sci@gmail.com","avatar":"https://avatars.githubusercontent.com/u/104419489?v=4"},"body":"> On Thu, Jun 29, 2023 at 9:33 AM Emily Shaffer wrote:\n\nHi, thanks for replying!\n\n> > Yeah, it's almost certainly stale in MyFirstObjectWalk - there was\n> > very recently a patch to clean up some headers which probably were\n> > implicitly including trace.h when I wrote this walkthrough. Patches\n> > totally welcome - and if you were working from the reference code in\n> > https://github.com/nasamuffin/git/tree/myfirstrevwalk\n>\n> bah, wrong link, the tutorial points to branch `revwalk` instead of\n> `myfirstrevwalk`, but the offer stands :)\n>\n> > and it's on your\n> > way to rebase and fix that too, I'm happy to update my branch\n> > accordingly too. (If you weren't, don't worry about doing the extra\n> > work, though.)\n\nSure will! But do you mean open a PR on your fork? I have the patch ready,\nand would be very happy to do so, if it is accepted!\n\nThanks a lot!\nVinayak\n"},{"id":"479026","messageId":"xmqq1qhu9ifp.fsf@gitster.g","threadId":"59930","inReplyTo":"CAJoAoZ=OEfsgkqsag926tH4GEuafX26A09SGZ1vR1uLh2W_4TA@mail.gmail.com","subject":"Re: Documentation/MyFirstObjectWalk: add #include \"trace.h\" to use trace_printf()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-29T19:28:42Z","receivedAt":"2023-06-29T19:31:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer writes:\n\n> Yeah, it's almost certainly stale in MyFirstObjectWalk - there was\n> very recently a patch to clean up some headers which probably were\n> implicitly including trace.h when I wrote this walkthrough.\n\nWe are lucky that we have folks like Vinayak who tried out the\nexamples and then bothered to spend time reporting the failure\ndiscovered. What does it take, however, for us to have a bit more\nautomated way to prevent such a breakage that comes from API\nchanges? Is it feasible, for example, to add a test that extracts\ncode snippets from the MyFirstObjectWalk document and try to build\nthe result? Alternatively, we can ship such a set of sample source\nfiles somewhere in our tree (e.g. contrib/examples?) and have such\na test try to build using the current set of source files, but then\nwe need a mechansim to ensure that the sample source files will not\ngo out of sync with the document.\n\nThoughts?\n\nThanks.\n\n"},{"id":"479035","messageId":"CAJoAoZnaU7WsCpnZY0Pvjg4_GJMZADF0FNC9fNZK56ShX2JO+g@mail.gmail.com","threadId":"59930","inReplyTo":"xmqq1qhu9ifp.fsf@gitster.g","subject":"Re: Documentation/MyFirstObjectWalk: add #include \"trace.h\" to use trace_printf()","fromName":"Emily Shaffer","fromEmail":"nasamuffin@google.com","sentAt":"2023-06-29T23:06:17Z","receivedAt":"2023-06-29T23:06:33Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Jun 29, 2023 at 12:28 PM Junio C Hamano wrote:\n>\n> Emily Shaffer writes:\n>\n> > Yeah, it's almost certainly stale in MyFirstObjectWalk - there was\n> > very recently a patch to clean up some headers which probably were\n> > implicitly including trace.h when I wrote this walkthrough.\n>\n> We are lucky that we have folks like Vinayak who tried out the\n> examples and then bothered to spend time reporting the failure\n> discovered. What does it take, however, for us to have a bit more\n> automated way to prevent such a breakage that comes from API\n> changes? Is it feasible, for example, to add a test that extracts\n> code snippets from the MyFirstObjectWalk document and try to build\n> the result? Alternatively, we can ship such a set of sample source\n> files somewhere in our tree (e.g. contrib/examples?) and have such\n> a test try to build using the current set of source files, but then\n> we need a mechansim to ensure that the sample source files will not\n> go out of sync with the document.\n\nYeah, I remember we talked about this when MyFirstContribution and\nMyFirstObjectWalk went in, but never made much headway. I do very much\nlike the idea of keeping the reference source in contrib/ as a set of\npatches, maybe along with a script to apply them (or a readme with the\nright `git am` invocation), and then checking that they still build.\nChecking that against the contents of the document is trickier,\nthough, like you mentioned. Hm.\n\nI'm interested in figuring out how to do this, but not likely to have\na lot of development time available to do it. Maybe I can take a day\nhere or there to poke at it, but if someone else is interested and\nbeats me to it, I will not be disappointed. :)\n\n>\n> Thoughts?\n>\n> Thanks.\n>\n"},{"id":"479036","messageId":"CAJoAoZ=X9hwZZ9eN2X=g04k2E6=wZsY1WEKFydMreNJKM3Mzng@mail.gmail.com","threadId":"59930","inReplyTo":"CADE8Naonm+bW_jVvJKmnfZWQyX=0-QVSHxpSaHs1qo+5DsCiPQ@mail.gmail.com","subject":"Re: Documentation/MyFirstObjectWalk: add #include \"trace.h\" to use trace_printf()","fromName":"Emily Shaffer","fromEmail":"nasamuffin@google.com","sentAt":"2023-06-29T23:09:11Z","receivedAt":"2023-06-29T23:09:27Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Jun 29, 2023 at 11:45 AM Vinayak Dev wrote:\n>\n> > On Thu, Jun 29, 2023 at 9:33 AM Emily Shaffer wrote:\n>\n> Hi, thanks for replying!\n>\n> > > Yeah, it's almost certainly stale in MyFirstObjectWalk - there was\n> > > very recently a patch to clean up some headers which probably were\n> > > implicitly including trace.h when I wrote this walkthrough. Patches\n> > > totally welcome - and if you were working from the reference code in\n> > > https://github.com/nasamuffin/git/tree/myfirstrevwalk\n> >\n> > bah, wrong link, the tutorial points to branch `revwalk` instead of\n> > `myfirstrevwalk`, but the offer stands :)\n> >\n> > > and it's on your\n> > > way to rebase and fix that too, I'm happy to update my branch\n> > > accordingly too. (If you weren't, don't worry about doing the extra\n> > > work, though.)\n>\n> Sure will! But do you mean open a PR on your fork? I have the patch ready,\n> and would be very happy to do so, if it is accepted!\n\nYeah, I think there are two things to fix:\n\nFirst, a patch to Documentation/technical/MyFirstObjectWalk.txt fixing\nthe snippets there. (I thought that was what you were offering to\npatch in your original mail, I may have been mistaken.)\n\nSecond, optionally, a rebased-and-fixed-and-your-attribution-added\nbranch of the reference impl that I can force-push to nasamuffin/git.\nThe more I think on it, I don't think the PR will help, since I will\nwant to just force-push that whole branch so the commit order still\nfunctions as a learning tool. So if you have it even in a branch on\nyour GitHub or GitLab fork, or a series of patches you'd want to mail\nto me, any of those are fine and I'll go ahead and rewrite my branch.\n\n\n>\n> Thanks a lot!\n> Vinayak\n"},{"id":"479041","messageId":"xmqqwmzl7krm.fsf@gitster.g","threadId":"59930","inReplyTo":"CAJoAoZnaU7WsCpnZY0Pvjg4_GJMZADF0FNC9fNZK56ShX2JO+g@mail.gmail.com","subject":"Re: Documentation/MyFirstObjectWalk: add #include \"trace.h\" to use trace_printf()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-30T02:21:17Z","receivedAt":"2023-06-30T02:21:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer writes:\n\n> ... I do very much\n> like the idea of keeping the reference source in contrib/ as a set of\n> patches, maybe along with a script to apply them (or a readme with the\n> right `git am` invocation), and then checking that they still build.\n> Checking that against the contents of the document is trickier,\n> though, like you mentioned. Hm.\n\nAn approach to avoid two things (i.e. sample source and the code\nsnippet in the documentation) going out of sync is not to have two\nof them in the first place. If we give up readability of the MFOW\ndocument in its source form, you may be able to arrange the code\nsnippet to be incorporated by the build test and documentation at\nthe same time.\n\n"},{"id":"479042","messageId":"CADE8NaqSn7DvvBHzLhGdWwZtwK+sxoKnVWo_G26P30p_WnVXJg@mail.gmail.com","threadId":"59930","inReplyTo":"CAJoAoZ=X9hwZZ9eN2X=g04k2E6=wZsY1WEKFydMreNJKM3Mzng@mail.gmail.com","subject":"Re: Documentation/MyFirstObjectWalk: add #include \"trace.h\" to use trace_printf()","fromName":"Vinayak Dev","fromEmail":"vinayakdev.sci@gmail.com","sentAt":"2023-06-30T05:48:26Z","receivedAt":"2023-06-30T05:48:57Z","isPatch":false,"sender":{"key":"vinayakdev.sci@gmail.com","avatar":"https://avatars.githubusercontent.com/u/104419489?v=4"},"body":"On Fri, 30 Jun 2023 at 04:39, Emily Shaffer wrote:\n> Yeah, I think there are two things to fix:\n>\n> First, a patch to Documentation/technical/MyFirstObjectWalk.txt fixing\n> the snippets there. (I thought that was what you were offering to\n> patch in your original mail, I may have been mistaken.)\n>\n> Second, optionally, a rebased-and-fixed-and-your-attribution-added\n> branch of the reference impl that I can force-push to nasamuffin/git.\n> The more I think on it, I don't think the PR will help, since I will\n> want to just force-push that whole branch so the commit order still\n> functions as a learning tool. So if you have it even in a branch on\n> your GitHub or GitLab fork, or a series of patches you'd want to mail\n> to me, any of those are fine and I'll go ahead and rewrite my branch.\n\nI will be pushing MyFirstObjectWalk's implementation to my Github fork,\nbut I might need a day's time to do that(I don't want to leave behind\nany mistakes).\nYou are right, a PR surely does not seem to be the best way to do this. As soon\nas I finish(shouldn't take too much time), I will reply with a link to\nthe branch.\nWould that be alright?\n\nThanks a lot!\nVinayak\n"},{"id":"479053","messageId":"CADE8NapQK2ouy4YDQA+3NNkUn_EegkSBQCtDfcSCVGmZvVufXg@mail.gmail.com","threadId":"59930","inReplyTo":"CADE8NaqSn7DvvBHzLhGdWwZtwK+sxoKnVWo_G26P30p_WnVXJg@mail.gmail.com","subject":"Re: Documentation/MyFirstObjectWalk: add #include \"trace.h\" to use trace_printf()","fromName":"Vinayak Dev","fromEmail":"vinayakdev.sci@gmail.com","sentAt":"2023-06-30T15:39:32Z","receivedAt":"2023-06-30T15:40:09Z","isPatch":false,"sender":{"key":"vinayakdev.sci@gmail.com","avatar":"https://avatars.githubusercontent.com/u/104419489?v=4"},"body":"Hi!\nI pulled down walken.c from https://github.com/nasamuffin/git/tree/revwalk\nand was able to fix the broken code. I also fixed\nDocumentation/MyFirstObjectWalk.txt\nand have accordingly pushed all the changes to my fork of git:\n\nhttps://github.com/vinayakdsci/git/tree/revwalk-fixed\n\nI had to remove init_walken_defaults() as I could not trace the\nfunction init_grep_defaults()\nwhich I think has been removed since you wrote the tutorial.\nI also didn't find a mention of init_grep_defaults() in the tutorial,\nso maybe that is alright.\nAlso, struct list_objects_filter_options is included inside of\nrev_info, so I don't think it requires initialisation\nany more.\n\nIt would be great if you are able to use this branch to rewrite your own!\n\nThanks a lot!\nVinayak\n"}]}