On Thu, Jan 4, 2024 at 2:24 AM Christian Couder <christian.couder@xxxxxxxxx> wrote: > > On Tue, Jan 2, 2024 at 11:17 PM Ghanshyam Thakkar > <shyamthakkar001@xxxxxxxxx> wrote: > > > > Hello, > > > > I'm currently an undergrad beginning my journey of contributing to the > > Git project. I am seeking feedback on doing "Heed core.bare from > > template config file when no command line override given" described > > here > > https://lore.kernel.org/git/5b39c530f2a0edf3b1492fa13a1132d622a0678e.1684218850.git.gitgitgadget@xxxxxxxxx/ > > by Elijah Newren, as a microproject. I would like to know from the > > community, if the complexity and scope of the project is appropriate > > for a microproject. > > Thanks for your interest in the next GSoC! > > My opinion is that it's too complex for a micro-project. Now maybe if > Elijah or others are willing to help you on it, perhaps it will work > out. I think it's safer to look at simpler micro-projects though. An important part of solving this problem, if you were to do so, would be adding several testcases (including some showing how it currently fails). Simply adding some or all of those testcases would be a good micro-project. (If you take this on, it'd probably be worthwhile to include a reference to any such added tests in the TODO comment here so that future folks noticing the TODO are aware of them). Then, if adding those testcases goes well and you feel ambitious, you can try to tackle the underlying problem too. > > e.g. in builtin/init-db.c : > > > > static int template_bare_config(const char *var, const char *value, > > const struct config_context *ctx, void *cb) > > { > > if(!strcmp(var,"core.bare")) { > > We like to have a space character between "if" and "(" as well as after a "," > > > is_bare_repository_cfg = git_config_bool(var, value); > > } > > return 0; > > } > > > > int cmd_init_db(int argc, const char **argv, const char *prefix) > > { > > ... > > ... > > if(is_bare_repository_cfg==-1) { > > We like to have a space character both before and after "==" as well > as between "if" and "(". > > > if(!template_dir) > > git_config_get_pathname("init.templateDir", > > &template_dir); > > > > if(template_dir) { > > const char* template_config_path > > = xstrfmt("%s/config", > > struct stat st; > > > > if(!stat(template_config_path, &st) && > > !S_ISDIR(st.st_mode)) { > > git_config_from_file(template_bare_cfg, > > template_config_path, NULL); > > } > > } > > ... > > ... > > return init_db(git_dir, real_git_dir, template_dir, hash_algo, > > initial_branch, init_shared_repository, flags); > > } > > > > I also wanted to know if the global config files should have an effect > > in deciding if the repo is bare or not. > > > > Curious to know your thoughts on, if this is the right approach or > > does it require doing refactoring to bring all the logic in setup.c. > > Based on your feedback, I can quickly send a patch. > > I don't know this area of the code well, so I don't think I can help > you much on this. If you look back at the mailing list discussion on the series that introduced this TODO comment you are trying to address, you'll note that both Glen and I dug into the code and attempted to explain it to each other, and we both got it wrong on our first try. If I knew the correct solution without digging, I probably would have just implemented it and sent it in as another patch series. My TODO was not meant to be a definitive guide about where to make the fixes (because I don't know yet), but just a helpful guide that the first spot you'd expect cannot be the correct location (I already tried a few things there) and that you need to dig further. Anyway, I'm sure the correct place to fix can be figured out with a bit of work, but in this case, figuring out where the changes need to be made is probably the majority of the effort. You may well need to just pick an area, start modifying, go through it in a debugger and with your testcases, etc., and learn whether modifications in that area even can solve the problem. You may have to discard your first or even second attempts, but take what you learned to guide you on future attempts. And if you do get it all working, this is a case where it'd likely be important to comment in the commit message not just why you are making changes, but why you believe the area you modified is the correct one (i.e. to mention why the more obvious places to modify don't actually solve the problem). And then be prepared for folks on the mailing list to use the information you provided in the commit message to dive in and point out some other way to fix the problem that is even better but requires you to redo it again. (And I'm not just saying that because you're new; I would not be surprised if the same happened to me. Others are more familiar with various parts of the general setup and config code than me, and sometimes additional expertise coupled with a solution of some sort can make it easier to identify an even better solution.)
