Repository.Submodules.Add created

[olmobrutall]

This is my first pull request. I've created an Add method to Submodules. I haven't been able to run xUnit on VS so there's no test.

Feel free to change it.

[dahlbyk]

I haven't been able to run xUnit on VS so there's no test.

Try the xUnit runner extension.

[dahlbyk]

use_gitLink can be bool.

[olmobrutall]

I've made all the changes but the Clone. Clone requires that there's no repository but when you add as submodule you have an empty one.

I've added the optional parameters exposed by fetch and checkout, so now you can control Tags, Progress, Credentials, etc...

[nulltoken]

@olmobrutall Hey! That looks like a nice addition to this project. Indeed, the submodule area need a bit of love. Thanks a lot for helping us with it!

Just a few thoughts.

• Unfortunately unit tests are not optional. Every new feature has to be covered with tests before the PR is merged. Have you considered testdriven.net? It's free for a open source developers. This should help you running the tests from within Visual Studio.
• Each PR is also tested thanks to Travis CI services. This one currently make at least one test fail. It's usually a good idea to open up the Travis build log to troubleshoot what made Travis unhappy 😉
• Currently the Submodules.Add() method deals with both the setup and the retrieval of the submodule content. I figure that soon we'll need a method that will only deal with the latter (eg. Submodules.GrabWhatChangedUpstream() or a better name...). I wonder if reducing the responsibility of Submodules.Add() to only setting things up wouldn't make the API a bit clearer.
• Maybe should we just think a bit ahead and come up with some prototype client code for the following standard recurring scenario
  • Fetch upstream changes of the submodule
  • Update the HEAD of the submodule to a specific commit (most of the time, this will be the latest but we also have to allow chosing any commit)
  • Stage the change of the HEAD of the submodule in the superproject

/cc @dahlbyk

[olmobrutall]

Hi @nulltoken,

• I will try to investigate the test suit and include some test for Submodule.Add, but I don't think I can put more time on it for now.

• Don't know about raw git commands (we use GitExtensions on a daily basis), but the current API maps nicely with my conceptual model of Submodules:

  • Adding a module doesn't make sense without a Fetch / Checkout, Staging however is a different thing.
  • Also libgit2 exposes two methods (git_submodule_add_setup and git_submodule_add_finalize) and expects you to Fetch in between.

• About the 'get latest version' feature using submodules. Take into account that git doesn't know the current branch in the submodule, just the commit SHA, so you need to specify the branch or commit anyway. This is the code I'm using to automatically update sub modules on a test project and push the super-project back to github:

  public static bool UpdateMusicSubmodules()
  {
      using (Repository rep = new Repository(Settings.Default.MusicProjectDirectory))
      {
          rep.Fetch("origin");
          rep.Branches["master"].Checkout();
          rep.Reset(ResetOptions.Hard, "origin/master");
  
          foreach (var sub in rep.Submodules)
          {
              using (Repository subRep = new Repository(Path.Combine(rep.Info.WorkingDirectory, sub.Path)))
              {
                  subRep.Fetch("origin");
                  subRep.Checkout("origin/master");
              }
  
              rep.Index.Stage(sub.Path);
          }
  
          var status = rep.Index.RetrieveStatus();
  
          if (status.Staged.Any())
          {
              Signature signature = new Signature(Settings.Default.GitCommitUsername, Settings.Default.GitCommitEmail, DateTimeOffset.Now);
              var commit = rep.Commit("Automatic update", signature, signature);
  
              rep.Network.Push(rep.Branches["master"], credentials: 
                  new Credentials { Username = Settings.Default.GitHubUsername, Password = Settings.Default.GitHubPassword });
  
              return true;
          }
  
          return false;
      }
  }
  

I think the API could be improved by:

• Having a Submodule.OpenSubRepository()

• Maybe having a proper Pull method on repository itself (i don't need Merge though).

  but I don't having a swiss knife method will add value.

Congratulations for this nicely designed piece of work, maps perfectly with Git concepts, to the point that is maybe a better way to introduce C# programmers to git without all the bash woodoo.

[nulltoken]

I wonder if reducing the responsibility of Submodules.Add() to only setting things up wouldn't make the API a bit clearer.

Adding a module doesn't make sense without a Fetch / Checkout

There would be a (convoluted) way to do this by retrieving the tips of the repository to submodule (through something similar to repo.Network.ListReferences()) and inserting in the index a submodule entry pointing to the Sha of one of the retrieved tips. But, you're very right, that wouldn't work if one's willing to point at a specific commit (or even use a revparse expression). Even if dislike this a bit, we'll have no choice than mimicking git.git process.

Having a Submodule.OpenSubRepository()

I like not having to combine the working directory path with the submodule location by hand, but I'm unsure about adding another way to instanciate a Repository? Could you please open a feature request so that we can discuss about this in a dedicated thread?

I've made all the changes but the Clone. Clone requires that there's no repository but when you add as submodule you have an empty one.

I'm not sure to follow you. Did you succeed in making it work? Do you encounter an issue but pushed the requested changes whatsoever? Could you please elaborate a bit on this, please?

Congratulations for this nicely designed piece of work, maps perfectly with Git concepts, to the point that is maybe a better way to introduce C# programmers to git without all the bash woodoo.

❤️ Thanks, but I don't deserve this. It's been a team work. Nothing would have been possible without all the awesome contributions of these people.

[ghost]

This is init-ing an empty repo at path, but at L57 here, you're attempting to clone into that directory. In order to clone into a directory, it must be empty.

[ghost]

I've made all the changes but the Clone. Clone requires that there's no repository but when you add as submodule you have an empty one.

I'm not sure to follow you. Did you succeed in making it work? Do you encounter an issue but pushed the requested changes whatsoever? Could you please elaborate a bit on this, please?

Check my comment here: https://github.com/libgit2/libgit2sharp/pull/482/files#r7454934

[pms1969]

Happy to take this up and finish the implementation. How do I get this patch into my own repo?

[nulltoken]

How do I get this patch into my own repo?

@pms1969 Add https://github.com/olmobrutall/libgit2sharp.git as a new remote of your repo, then fetch from it.

[nulltoken]

@pms1969 Some more gory details

• The libgit2 submodule.h provides extensive documentation about the available functions.
• The submodule tests are also quite handy to understand how those functions behave.

[pms1969]

@nulltoken

You're a champ.Thanks. I'll see what I can manage.

[pms1969]

I'm getting the following 2 errors when trying to compile..... Should I be using the "Lax" or "Strict" versions?

Error 1 The type or namespace name 'Utf8Marshaler' could not be found (are you missing a using directive or an assembly reference?) C:\Development\libgit2sharp\LibGit2Sharp\Core\NativeMethods.cs 1294 122 LibGit2Sharp
Error 2 The type or namespace name 'FilePathMarshaler' could not be found (are you missing a using directive or an assembly reference?) C:\Development\libgit2sharp\LibGit2Sharp\Core\NativeMethods.cs 1295 122 LibGit2Sharp

[nulltoken]

Hmmm. It'll be hard to troubleshoot those errors without peeking at the code, could you please push it in a branch an open a PR?

Regarding the Lax/Strict Marshalers, we use them as follows:

• When the data being marshaled comes from the user, we use the StrictMarshaler.
• When the data being marshaled comes from libgit2, we use the LaxMarshaler.

[pms1969]

I had set them to Lax, but I think I may need to set them to Strict. It compiles now, but alas, I am at work. I plan to get the unit testing working on the way home, and set up a test or 2. Then I'll push and open a pull request. (might be next week now tho, since Easter is going to get in the way of anything productive).

[pms1969]

What branch should I push it too? any name do?

[nulltoken]

What branch should I push it too? any name do?

The branch name doesn't matter much (we tend to use something that identifies the feature eg. submodule_add). It's only a way to keep your work isolated from vNext and master. Thus, when some more commits are added upstream on those branches, it's easier for you to rebase your work onto them.

[JasonGoemaat]

I wouldn't mind taking this over as I need this functionality for a project I'm working on converting from another source control provider to git. I just have one issue that I'm not sure about.

I can't find in the libgit2 api how you can add a submodule specifying a name that is different than the path:

GIT_EXTERN(int) git_submodule_add_setup(
    git_submodule **out,
    git_repository *repo,
    const char *url,
    const char *path,
    int use_gitlink);

Does it make sense then for SubmoduleCollection.Add() to only take the path and not the name then? The submodule ends up getting named whatever the relative path is.

[carlosmn]

This is not functionality that belongs in SubmoduleCollection which should be about managing them, rather than emulating git's higher-level commands.

As this rather old, I'm going to close it, but if you still want to, something like this would belong in Commands.