Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support git submodules as dependencies #1840

Open
Geod24 opened this issue Dec 27, 2019 · 18 comments
Open

Support git submodules as dependencies #1840

Geod24 opened this issue Dec 27, 2019 · 18 comments

Comments

@Geod24
Copy link
Member

Geod24 commented Dec 27, 2019

It has been a desire of mine for some time to have proper git submodule support.
Since I haven't seen a proper specification on what such an integration would look like for the end user, I will attempt to do so here. Apologies for the wall of text.

For dub to interact "properly" with git submodule, it should be able to do the following:

  • Recognize the tag of a submodule and use it in version resolution, erroring out if the version does not match the requirement in the manifest file;
  • Automatically fetch dependencies on dub build (or test / run);
  • dub upgrade should fetch the submodules' default remote and git checkout properly;
  • dub add should support adding a submodule, either via name (with an option), or via explicit git URL;
  • dub remove can either remove the submodule, or just its registration (to allow some testing);
  • dub.settings.json should have a key to support this as the default mechanism;
  • The version should still be recorded in dub.selections.json to support the registry (Github release don't include submodules);
  • When git adding a submodule which has submodules (e.g. vibe.d), the submodules are added directly to the project. Recursive submodules would create issues, so submodules should ALWAYS be flattened. We can offer an option not to flatten, in which case submodules are not used for dependencies of dependencies, but it should be the default to flatten.

The main issue to consider is what happens if I add two libraries as dependency which have submodules. If those libraries have committed their dub.selections.json, the exact submodule version will be in it, and this can be used for version resolution. If not, the submodule is checked out in the current project, and the version resolution takes place normally (as both libraries' manifest will include a version indication).

An interaction that needs thought is dub run: Since Github release does not include submodule content, dub run would not work iff someone used a submodule not on the registry.
However, the reason to allow dub add with a git URL is to support overriding dependencies (e.g. when one needs to adapt Vibe.d to one's need and can't upstream / need to iterate fast), so the registry could simply reject any package where a submodule is not registered in it.
Note that the match in the registry would also most likely need to be name-based (not URL based), so that multiple registries are supported (with a local registry overriding the global one).

Submodules should behave roughly like a local cache. The default checkout place could be .dub/packages/, however that conflicts with the default .gitignore, so open to discussion.

An example of what we currently do, using path-based dependency only:

    "dependencies": {
        "base32":           { "path": "submodules/base32/", "version": "*" },
        "bitblob":          { "path": "submodules/bitblob/", "version": "*" },
        "d2sqlite3":        { "path": "submodules/d2sqlite3/", "version": "*" },
        "dyaml":            { "path": "submodules/dyaml/", "version": "*" },
        "libsodiumd":       { "path": "submodules/libsodiumd/", "version": "*" },
        "localrest":        { "path": "submodules/localrest/", "version": "*" },
        "ocean":            { "path": "submodules/ocean/", "version": "*" },
        "protobuf":         { "path": "submodules/protobuf-d/", "version": "*" },
        "vibe-d":           { "path": "submodules/vibe.d/", "version": "*" }
    }
{
	"fileVersion": 1,
	"versions": {
		"base32": { "path": "submodules/base32/" },
		"bitblob": { "path": "submodules/bitblob/" },
		"botan": { "path": "submodules/botan/" },
		"botan-math": { "path": "submodules/botan-math/" },
		"d2sqlite3": { "path": "submodules/d2sqlite3/" },
		"diet-ng": { "path": "submodules/diet-ng/" },
		"dyaml": { "path": "submodules/dyaml/" },
		"eventcore": { "path": "submodules/eventcore/" },
		"libasync": { "path": "submodules/libasync/" },
		"libevent": { "path": "submodules/libevent/" },
		"libsodiumd": { "path": "submodules/libsodiumd/" },
		"localrest": { "path": "submodules/localrest/" },
		"memutils": { "path": "submodules/memutils/" },
		"mir-linux-kernel": { "path": "submodules/mir-linux-kernel/" },
		"openssl": { "path": "submodules/openssl/" },
		"protobuf": { "path": "submodules/protobuf-d/" },
		"stdx-allocator": { "path": "submodules/stdx-allocator/" },
		"taggedalgebraic": { "path": "submodules/taggedalgebraic/" },
		"tinyendian": { "path": "submodules/tinyendian/" },
		"vibe-core": { "path": "submodules/vibe-core/" },
		"vibe-d": { "path": "submodules/vibe.d/" },
	}
}

Example syntax for dub.json:

    "dependencies": {
         // This ?
        "vibe-d":           { "mode": "submodule", "version": ">=0.7.42" },
        // With an optional path
        "vibe-d":           { "mode": "submodule", "path": "submodules/someotherpath", "version": ">=0.42" }
    }

In order for this to work nicely, #1706 needs to be fixed, as currently it would add a lot of packages as submodules.
Also note that we might be able to do away with the "mode": "submodule" if we implement a matching between the .gitmodules and the name of the dependency.
Version wise, we can support intermediate commits for development purpose, but any package not depending on a tagged version would be rejected by the registry.

TL;DR: First class submodules support, submodules are never used recursively, only flattened, submodules that are not registered on the registry are only available for development use cases, submodules URL is not included in dub.json / dub.selections.json.

Supersedes #1044
Previous work / interest / use cases:

@FeepingCreature : I'm especially interested to know if this would fulfill all of Funkwerk's needs.
@belka-ew : Since you had interest in a similar feature, would this proposal fulfill your use case ? This explicitly exclude having packages on the registry that have non-registry-registered submodules, but everything else should work.

@belka-ew
Copy link
Contributor

belka-ew commented Jan 2, 2020

Proposal to have the URL in the manifest: #104 . This is slightly different, as the current proposal explicitly exclude URL from the manifest, as we don't want any package in the registry that depends on package outside of the registry (which this would allow). It was implemented in #1765 but later reverted.

I reopened my PR, that was reverted: #1798, but still have to finish it.
I'm working on this feature on behalf of Funkwerk as well. And submodules are a different story, they don't solve this problem.

Since you had interest in a similar feature, would this proposal fulfill your use case ? This explicitly exclude having packages on the registry that have non-registry-registered submodules, but everything else should work.

The ability to specify the dependencies as URL is intended only for testing/development. We have a private registry, where we host public and private packages. The idea is to be able to test the package in a "bigger" system before tagging and uploading it to the registry. For example if I'm working on a package MyPackage, I'd like to test it in the program MyProgram. After I see, that my program works with MyPackage as expected, I make a new tag in MyPackage and put it into the registry and replace the git URL in MyProgram with the version. I actually agree that Git URLs shouldn't be allowed in the registry but I still find the feature useful for local development.

@Geod24
Copy link
Member Author

Geod24 commented Jan 2, 2020

The ability to specify the dependencies as URL is intended only for testing/development. We have a private registry, where we host public and private packages. The idea is to be able to test the package in a "bigger" system before tagging and uploading it to the registry. For example if I'm working on a package MyPackage, I'd like to test it in the program MyProgram. After I see, that my program works with MyPackage as expected, I make a new tag in MyPackage and put it into the registry and replace the git URL in MyProgram with the version. I actually agree that Git URLs shouldn't be allowed in the registry but I still find the feature useful for local development.

If I'm not mistaken, this could be possible with the above proposal.

# In MyPackage
$ git describe HEAD
v41.0.0
$ git commit -am "Beta version for v42.0.0
$ git push # If you want other people to try it, otherwise you can just add your dev folder as a remote
# In MyProgram
$ git submodules
2a2a2a2a2a2a2a submodules/MyPackage (v41.0.0)
$ dub upgrade MyPackage --prerelease
# Performs 'git -C submodules/MyPackage fetch' which retrieves your commit
MyPackage have been updated to v41.0.1-1-g424242

Now on a clean machine, without submodules:

# In MyProgram
$ dub upgrade MyPackage --prerelease
# Nothing happens, v41.0.0 is the latest version in the registry

@Geod24
Copy link
Member Author

Geod24 commented Jan 2, 2020

MyPackage have been updated to v41.0.1-1-g424242

By the way, that will need to be a hack on dub's side. The version that git provides is actually "v41.0.0-1-g424242", and according to SemVer that compares less than v41.0.0 (because it's a pre-release).
Not sure what would be the best way to handle this though (I made a PoC implementation to try to find that kind of shortcomings).

@belka-ew
Copy link
Contributor

belka-ew commented Jan 2, 2020

If I'm not mistaken, this could be possible with the above proposal.

MyPackage is not a submodule of MyProgram, it is a normal dependency in dub.json; I just want to force a specific commit for a short test and in-place editing, until it works. But yes, I can actually add MyPackage as submodule and remove it afterwards. Seems not to be inconvinient, just adds an additional step.

By the way, that will need to be a hack on dub's side. The version that git provides is actually "v41.0.0-1-g424242", and according to SemVer that compares less than v41.0.0 (because it's a pre-release).

Something I never thought about. It probably doesn't look fine already for paths. If a path has the .git directory, dub determines the version as 0.10.3+commit.2.g4dc960f which as far as I see has the same ordering in SemVer as 0.10.3 despite being a few commits ahead. Probably similar thing can be used for the submodules too. Imho submodules and paths should behave similarly.

@Geod24
Copy link
Member Author

Geod24 commented Jan 2, 2020

Good point! Turning v41.0.0-1-g424242 into v41.0.0+1-g424242 sounds like it could work without drawback.

MyPackage is not a submodule of MyProgram, it is a normal dependency in dub.json;

Right. The underlying idea for this workflow is also to allow small organizations (like mine) to cooperate without setting up a dub registry unless they have external users, while still using dub. Otherwise everything can be handled through git / github. But I suppose for you the workflow would be a bit less smooth.

@LunaTheFoxgirl
Copy link

For https://github.com/Inochi2D/bindbc-imgui we need to be able to fetch from git + all of the submodules for it recursively, as imgui's dependency tree is a spaghetti monster beyond comprehension. imgui and all of the dependencies are written in C++ and our dub package requires a C++ compiler + cmake installed, which makes things more difficult.

@rikkimax
Copy link
Contributor

For https://github.com/Inochi2D/bindbc-imgui we need to be able to fetch from git + all of the submodules for it recursively, as imgui's dependency tree is a spaghetti monster beyond comprehension. imgui and all of the dependencies are written in C++ and our dub package requires a C++ compiler + cmake installed, which makes things more difficult.

For that particular use case, it is as simple as adding the right arguments to[0]. However, that is not what @Geod24 was asking for here initially. So need more info about how to go from here for the more complex support.

[0]

string[] args = ["git", "clone", "--no-checkout"];

@WebFreak001
Copy link
Member

I don't really see any disadvantage in cloning recursively from the start, should we just add it?

In the future we might consider adding cloning dependencies that don't need a dub.json at all (to import non-D projects) but for now it's possible to work around that limitation with a simple wrapper repository that's just an empty sourceLibrary or none package I guess.

@rikkimax
Copy link
Contributor

I don't really see any disadvantage in cloning recursively from the start, should we just add it?

I'm certainly partial to adding it. I'm just worried that it might lead to breaking changes later on if we get better support, so want to confirm that first.

@WebFreak001
Copy link
Member

do you maybe have some more concrete example what could break?

@Geod24
Copy link
Member Author

Geod24 commented Jun 14, 2022

I don't really see any disadvantage in cloning recursively from the start, should we just add it?

I do. If you look at sociomantic-tsunami projects, we never had recursive clone, because it's much easier to keep all your dependencies at the same (top) level. Recursive submodules are a real pain.

@FeepingCreature
Copy link
Contributor

FeepingCreature commented Jun 14, 2022

Same, but we do our own builds anyway so...

The problem with recursive clones is if you keep the API of a service in the same repo, then any attempt to include it as a dependency will recursively draw in ~10 copies of build tools, utility libraries, database libraries, etc. So for that usecase, recursive submodules are just a bad idea.

Submodules are not a package management system, but if you clone them flat you can sort of pretend that they are. With deeper clones, that goes out the window.

@LunaTheFoxgirl
Copy link

In Inochi2D's case getting all the libraries down is neccesary for the various build modes, since they don't purely rely on the D compiler, we run a C++ compiler during the preBuildCommands to build stuff like cimgui with the exact configuration our project needs, with an easy path to just update the submodules to keep up to date with upstream imgui.

@WebFreak001
Copy link
Member

if there are split opinions I think we could just add a boolean setting to git dependencies to clone them recursively and leave it off by default. (less by default)

@LunaTheFoxgirl
Copy link

Would be double nice if you could define that a dub package should be cloned (from tag) via git instead of downloading the zip...

@WebFreak001
Copy link
Member

you can do that, see https://dlang.org/changelog/2.094.0.html#git-paths

@LunaTheFoxgirl
Copy link

LunaTheFoxgirl commented Jun 14, 2022

No I mean as in the dub package on code.dlang.org just fetching the named dependency as a git clone instead.
Because the alternative is taking our package down from dub since right now it will fail to build due to the missing submodules not being there to build the C++ dependency.

@WebFreak001
Copy link
Member

right, right now when there is a recursive flag it could be worked around with by providing a simple targetType: none dub package with git submodules that is only ever added as dependency to other dub projects through a git url, but having control over it in the registry eventually would be nice to have. I think that's a whole separate issue that should be discussed in the dub-registry repo though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants