Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Hi Samo, On Mon, Jun 24, 2024 at 04:53:01PM +0200, Samo Pogačnik wrote: > i prepared another candidate for the 0.4.6-1 release ( > https://salsa.debian.org/spog/git-subrepo/-/commit/f96eeedd0e96b6f2bbcc8c013909de5d5325cafe > ), hoping it ticks all the boxes and more :) Excellent! I did run into some more issues during build/testing unfortunately. Some beurocratic, some related to tests: - you are still missing from d/copyright :) - d/changelog should not contain versions that were never in Debian. consequently everything that isn't the "Intial release" entry also doesn't make sense. Not sure why I didn't catch that before. - tests fail in a git checkout (but not during clean sbuild it seems) - d/rules clean doesn't cleanup after tests The package looks good sans tests so I'll upload that to NEW so we can keep working on the tests in the meantime. Remeber to pull my changes from debian/git-subrepo before you continue working :) For the cleanup requirement see https://www.debian.org/doc/debian-policy/ch-source.html#main-building-script-debian-rules > clean (required) > >This must undo any effects that the build and binary targets may have >had, except that it should leave alone any output files created in the >parent directory by a run of a binary target. Right now I have to `git reset --hard HEAD; git clean -xfd` to get rid of all the build byproducts. This is one of the more onerous requirements of Debian policy rooted in a world that didn't have git yet. It's important because some people still work on packages without git and they need a way to reset the build tree. Looking at what remains after `quilt pop -a` I see untracked files test/repo/* which you create with your 0005 patch. Really that should probably be in test/tmp which upstream has already setup to be removed by their clean target. Other than that I see changes to upstream files: modified test/repo/bar/config @@ -2,3 +2,7 @@ repositoryformatversion = 0 filemode = true bare = true + logallrefupdates = true +[user] + name = BarUser + email = bar@bar deletedtest/repo/bar/objects/94/c86ffc745232d89f78c6f895e11e71272518db deletedtest/repo/bar/objects/f6/2a8ff3feadf39b0a98f1a86ec6d1eb33858ee9 modified test/repo/bar/refs/heads/master @@ -1 +1 @@ -94c86ffc745232d89f78c6f895e11e71272518db +e2aebf65b96f0be7595bffed8ff42cc69334f150 modified test/repo/bar/refs/tags/A @@ -1 +1 @@ -f62a8ff3feadf39b0a98f1a86ec6d1eb33858ee9 +f8206189232afa81e999b1b3aeb524fc4d9bae46 modified test/repo/foo/config @@ -2,3 +2,7 @@ repositoryformatversion = 0 filemode = true bare = true + logallrefupdates = true +[user] + name = FooUser + email = foo@foo deletedtest/repo/foo/objects/e2/1291a1ad392a9d4c51dd9586804f1467b28afd modified test/repo/foo/refs/heads/master @@ -1 +1 @@ -e21291a1ad392a9d4c51dd9586804f1467b28afd +e51641c054bb9e0591a8a95d0789e10abb308de2 modified test/repo/init/config @@ -1,5 +1,8 @@ [core] repositoryformatversion = 0 filemode = true - bare = false + bare = true logallrefupdates = true +[user] + name = InitUser + email = init@init deletedtest/repo/init/objects/32/5180321750a21cd7a4e7ecda319e557a4f6a09 deletedtest/repo/init/objects/3d/918c6901c02f43af5d31779dd5e1f9166aeb36 deletedtest/repo/init/objects/58/931fc1bd559b59c41ea738fc7ad04f9ad01bd3 deletedtest/repo/init/objects/94/7b3d714c38791e95ad6f928b48c98bb8708acd deletedtest/repo/init/objects/c3/ee8978c4c5d84c3b7d00ba8e5906933d027882 modified test/repo/init/refs/heads/master In general you must not change upstream files during the Debian package build. Sometimes this is necessary, in such cases a save+restore approach is taken cf. dh_autoreconf. However this is not appropriate here. Also, looking at 0005: > diff --git a/Makefile b/Makefile > index f84b83d..1927073 100644 > --- a/Makefile > +++ b/Makefile > @@ -47,6 +47,18 @@ help: > @echo 'uninstall Uninstall $(NAME)' > @echo 'envShow environment variables to set' > > +.PHONY: genfoo > +genfoo: > + test/genfoo test/repo > + > +.PHONY: genbar > +genbar: > + test/genbar test/repo > + > +.PHONY: geninit > +geninit: > + test/geninit test/repo > + > .git: > git init -b main . > git config user.email "you@you" > @@ -57,6 +69,9 @@ help: > > .PHONY: test > test: .git > + make genfoo > + make genbar > + make geninit > prove $(prove) $(test) Recursive make calls are uncessary here, but you're also ignoring the arcane convention to call $(MAKE), see https://www.gnu.org/software/make/manual/html_node/MAKE-Variable.html. Easy fix is to just add dependencies to the test target, like so: .PHONY: test -test: .git +test: .git genfoo genbar geninit prove $(prove) $(test) Upstream's suggestion to put it in test/setup is even better though so that's the way to go. > I made the PR
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Hi Daniel, i prepared another candidate for the 0.4.6-1 release ( https://salsa.debian.org/spog/git-subrepo/-/commit/f96eeedd0e96b6f2bbcc8c013909de5d5325cafe ), hoping it ticks all the boxes and more:) I made the PR upstream (https://github.com/ingydotnet/git-subrepo/pull/623) and added 'Forwarded' fields with PR to all five patches. I hope that making a separate commit for adding 'Forwarded' field to patches is ok. The 'git-subrepo.d' subdir has been removed upon 'make install' and additional Makefile adjustment done according to your suggestions. Regarding internal test suite, things weren't that trivial, as the implementation requires that the project is a git worktree. However debian builds from a non-git tarball. I fixed that by git-initializing the project on the fly, when needed as well as providing local git configuration for all repos involved in tests. Another thing regarding tests was that they rely upon english output, so tests failed in reprotest, when executed in a French environment for example. By setting fixed locale during each test being run, it all went well. I also went a step further regarding tests. Three test repos being used by several tests were committed upstream as binary bare git repos, which i really do not like. So i prepared a few scripts, which generate the same repos with the same history upon each test suite startup. best regards, Samo
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Hi Samo, On Fri, Jun 14, 2024 at 09:55:31PM +0200, Samo Pogačnik wrote: > Hi Daniel, > > Dne 14.06.2024 (pet) ob 20:50 +0200 je Daniel Gröber napisal(a): > > Below you're not ACK'ing some of my comments again. With email review you > > really kind of have to say something about each comment otherwise it's > > really hard to tell if you just missed one or not and it's easier to > > discuss any disagreements sooner (when I have forgotten less of the context > > :x) > > rather than later when you send the next version. > > I just realized that i sent the unfinished mail instead of saving the > draft. I am sorry. I'll add the missing replies here and thanks for the > valuable response. No worries, that happens :) > > I noticed another thing, w > > e disable the test suite for what appear to be > > trivial reasons. I really don't like maintaining packages without a test > > suite so this is a no-go. Please look into why it's not working and fix it > > with more upstreamable patches if necessary. > > > > From a quick look it seems removing the `git config core.autocrlf input` > > call in test/setup already gets us quite far but the way git subrepo finds > > its libs needs adjustment too. > > > > Commit review below: > > > > > From: Samo Pogačnik > > > > > > Default 'git-core' location of git-subrepo executables currently > > > > +The default 'git-core' ... ? > thanks > > > > > > does not automatically integrate git-subrepo into git's own bash- > > > completion. This change moves git-subrepo executables into > > > /usr/share/git-subrepo and adds a symlink of its main executable > > > script into /usr/bin to achieve recognition of the 'git subrepo' > > > sub-command under the git bash-completion (through git's: > > > --list-cmds=...,other,...). > > > > > > Gbp-Pq: Name 0001-Fixed-bash-completion-integration-with-git.patch > > > --- > > > Makefile | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/Makefile b/Makefile > > > index 79898f5..3d84454 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -17,7 +17,7 @@ SHARE = share > > > > > > # Install variables: > > > PREFIX ?= /usr/local > > > -INSTALL_LIB ?= $(DESTDIR)$(shell git --exec-path) > > > +INSTALL_LIB ?= $(DESTDIR)$(PREFIX)/share/$(NAME) > > > > While we're fixing upstream's mess $(DESTDIR) should be interpolated in the > > install target only so overriding the directory structure is easier. > > > > The install target should look something like this, vars listed for > > clarity: > > > > ``` > > PREFIX ?= /usr/local > > INSTALL_LIB ?= $(PREFIX)/share/$(NAME) > > install: > > $(INSTALL) -d -m 0755 $(DESTDIR)$(INSTALL_LIB)/ > > $(INSTALL) -C -m 0755 $(LIB) $(DESTDIR)$(INSTALL_LIB)/ > > ``` > > > > Notice the canonical use of $(INSTALL) instead of the plain command, > > doesn't make a difference in this case but it's good Makefile hygiene. > > > ACK > > > With that setup we can just export the INSTALL_* vars in debian/rules to > > override them: > > > > export INSTALL_LIB=/usr/share/git-subrepo > > export INSTALL_EXT=$(INSTALL_LIB) > > > > Setting INSTALL_EXT equal to INSTALL_LIB gets rid of the git-subrepo.d as > > I've been requesting. > > > > I'm sending you the full patch "Drop unecessary subdir in usr/share" I used > > to test this seperately, lets see if you can figure out how to apply it to > > your repo ;) > > > ACK > > > You still have to add a seperate commit to make the $(DESTDIR) adjustment. > > > > > INSTALL_EXT ?= $(INSTALL_LIB)/$(NAME).d > > > INSTALL_MAN1 ?= $(DESTDIR)$(PREFIX)/share/man/man1 > > > > > > @@ -67,6 +67,8 @@ install: > > > install -C -m 0755 $(EXTS) $(INSTALL_EXT)/ > > > install -d -m 0755 $(INSTALL_MAN1)/ > > > install -C -m 0644 $(MAN1)/$(NAME).1 $(INSTALL_MAN1)/ > > > + install -d -m 0755 $(DESTDIR)$(PREFIX)/bin/ > > > + ln -s ../share/$(NAME)/$(NAME) $(DESTDIR)$(PREFIX)/bin/$(NAME) > > > > Creating symlinks like this we'd usually do with dh_link(1). This would be > > OK if you're intending to send this patch upstream? > > > ACK > > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -64,7 +64,7 @@ install: > > > install -C -m 0755 $(LIB) $(INSTALL_LIB)/ > > > sed -i 's!^SUBREPO_EXT_DIR=.*!SUBREPO_EXT_DIR=$(INSTALL_EXT)!' > > > $(INSTALL_LIB)/$(NAME) > > > install -d -m 0755 $(INSTALL_EXT)/ > > > - install -C -m 0755 $(EXTS) $(INSTALL_EXT)/ > > > + install -C -m 0644 $(EXTS) $(INSTALL_EXT)/ > > > > This sort of thing would usually be done in debian/rules to keep things > > maintainable. Again if the patch is intended to go upstream it's > > acceptable. > > > ACK > > > > diff --git a/lib/git-subrepo.d/help-functions.bash > > > b/lib/git-subrepo.d/help- > > > functions.bash > > > index 123bb54..6dd5c17 100644 > > > --- a/lib/git-subrepo.d/help-functions.bash > > > +++ b/lib/git-subrepo.d/help-functions.bash > > > @@ -1,5 +1,3 @@ > > > -#!/usr/bin/env bash > > > - > > > # DO NOT EDIT. This file
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Hi Daniel, Dne 14.06.2024 (pet) ob 20:50 +0200 je Daniel Gröber napisal(a): > > Below you're not ACK'ing some of my comments again. With email review you > really kind of have to say something about each comment otherwise it's > really hard to tell if you just missed one or not and it's easier to > discuss any disagreements sooner (when I have forgotten less of the context > :x) > rather than later when you send the next version. > I just realized that i sent the unfinished mail instead of saving the draft. I am sorry. I'll add the missing replies here and thanks for the valuable response. > I noticed another thing, w > e disable the test suite for what appear to be > trivial reasons. I really don't like maintaining packages without a test > suite so this is a no-go. Please look into why it's not working and fix it > with more upstreamable patches if necessary. > > From a quick look it seems removing the `git config core.autocrlf input` > call in test/setup already gets us quite far but the way git subrepo finds > its libs needs adjustment too. > > Commit review below: > > > From: Samo Pogačnik > > > > Default 'git-core' location of git-subrepo executables currently > > +The default 'git-core' ... ? thanks > > > does not automatically integrate git-subrepo into git's own bash- > > completion. This change moves git-subrepo executables into > > /usr/share/git-subrepo and adds a symlink of its main executable > > script into /usr/bin to achieve recognition of the 'git subrepo' > > sub-command under the git bash-completion (through git's: > > --list-cmds=...,other,...). > > > > Gbp-Pq: Name 0001-Fixed-bash-completion-integration-with-git.patch > > --- > > Makefile | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/Makefile b/Makefile > > index 79898f5..3d84454 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -17,7 +17,7 @@ SHARE = share > > > > # Install variables: > > PREFIX ?= /usr/local > > -INSTALL_LIB ?= $(DESTDIR)$(shell git --exec-path) > > +INSTALL_LIB ?= $(DESTDIR)$(PREFIX)/share/$(NAME) > > While we're fixing upstream's mess $(DESTDIR) should be interpolated in the > install target only so overriding the directory structure is easier. > > The install target should look something like this, vars listed for > clarity: > > ``` > PREFIX ?= /usr/local > INSTALL_LIB ?= $(PREFIX)/share/$(NAME) > install: > $(INSTALL) -d -m 0755 $(DESTDIR)$(INSTALL_LIB)/ > $(INSTALL) -C -m 0755 $(LIB) $(DESTDIR)$(INSTALL_LIB)/ > ``` > > Notice the canonical use of $(INSTALL) instead of the plain command, > doesn't make a difference in this case but it's good Makefile hygiene. > ACK > With that setup we can just export the INSTALL_* vars in debian/rules to > override them: > > export INSTALL_LIB=/usr/share/git-subrepo > export INSTALL_EXT=$(INSTALL_LIB) > > Setting INSTALL_EXT equal to INSTALL_LIB gets rid of the git-subrepo.d as > I've been requesting. > > I'm sending you the full patch "Drop unecessary subdir in usr/share" I used > to test this seperately, lets see if you can figure out how to apply it to > your repo ;) > ACK > You still have to add a seperate commit to make the $(DESTDIR) adjustment. > > > INSTALL_EXT ?= $(INSTALL_LIB)/$(NAME).d > > INSTALL_MAN1 ?= $(DESTDIR)$(PREFIX)/share/man/man1 > > > > @@ -67,6 +67,8 @@ install: > > install -C -m 0755 $(EXTS) $(INSTALL_EXT)/ > > install -d -m 0755 $(INSTALL_MAN1)/ > > install -C -m 0644 $(MAN1)/$(NAME).1 $(INSTALL_MAN1)/ > > + install -d -m 0755 $(DESTDIR)$(PREFIX)/bin/ > > + ln -s ../share/$(NAME)/$(NAME) $(DESTDIR)$(PREFIX)/bin/$(NAME) > > Creating symlinks like this we'd usually do with dh_link(1). This would be > OK if you're intending to send this patch upstream? > ACK > > --- a/Makefile > > +++ b/Makefile > > @@ -64,7 +64,7 @@ install: > > install -C -m 0755 $(LIB) $(INSTALL_LIB)/ > > sed -i 's!^SUBREPO_EXT_DIR=.*!SUBREPO_EXT_DIR=$(INSTALL_EXT)!' > > $(INSTALL_LIB)/$(NAME) > > install -d -m 0755 $(INSTALL_EXT)/ > > - install -C -m 0755 $(EXTS) $(INSTALL_EXT)/ > > + install -C -m 0644 $(EXTS) $(INSTALL_EXT)/ > > This sort of thing would usually be done in debian/rules to keep things > maintainable. Again if the patch is intended to go upstream it's > acceptable. > ACK > > diff --git a/lib/git-subrepo.d/help-functions.bash b/lib/git-subrepo.d/help- > > functions.bash > > index 123bb54..6dd5c17 100644 > > --- a/lib/git-subrepo.d/help-functions.bash > > +++ b/lib/git-subrepo.d/help-functions.bash > > @@ -1,5 +1,3 @@ > > -#!/usr/bin/env bash > > - > > # DO NOT EDIT. This file generated by pkg/bin/generate-help-functions.pl. > > Should we not fix pkg/bin/generate-help-functions.pl too/instead ? > > The header does say not to edit this ;) > While, the released code contains both the generated code and the generators, and it seem's that there is no need to call generators downstream, i corrected both (see below). > > > > set -e > >
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Hi Samo, On Fri, Jun 14, 2024 at 07:15:40PM +0200, Samo Pogačnik wrote: > Thanks for the review. I'll do my best to include as much as i know how > to into another 'release candidate', however it may take me a while. Thanks for working on this :) Recent discussion on d-devel makes me think git-subtree/subrepo have a good chance of becoming more popular as we educate upstreams about how to properly avoid future xz snafu. So you're doing really important work here <3 > Dne 10.06.2024 (pon) ob 22:26 +0200 je Daniel Gröber napisal(a): > > In general you're missing the Debian patch metadata, see [DEP-3]. I hate > > this archaic stuff I just mention it for completness. If you use gbp-pq for > > generating the patches you can mostly ignore. I do like to use the > > Forwarded field to note the upstream PR for the patch tho. > > > > [DEP-3]: https://dep-team.pages.debian.net/deps/dep3/ > > > > Next time I see the patches I want to see a Forwarded field for each -- one > > PR for all of them pleas, not spam upstream :) > > > I am a bit confused. I'm not sure i want to present patches upstream while you > (at least) have not yet accepted them. Otherwise i'd have to revise my > upstream > proposal (and spam upstream) every time you find another thing i missed about > the debian policy (which i probably will for some time). How do you see this? That's not spamming upstream so much as just doing upstream development :) There's nothing wrong with pushing revisions in that case. If you insist you can send me the patches/branch for review first, but I don't see a need for that. I was just trying to be clear about the fact that these patches should be well motivated and single-purpose so upstream can understand them. If it happens that upstream and Debian policy disagree there's no problem with sending upstream the patches they'll take and just add the rest on top in the Debian package patch stack. I do think we should try to always push patches upstream first to keep them informed about what we're doing. The "spam" comment was really just me making sure you're aware you're not expected to split each patch into it's own PR. Don't worry about that too much. Looking at the upstream activity I fear us being ignored may the worst that'll happen. My hunch is that we may end up having to take over as git-subrepo upstream. Only one way to know (send a PR) and in any case we'd want clean commits :) One workflow note: whatever you do you should make sure not to have Gbp-* fields in the commits you send upstream. Getting the Debian tooling to cooperate here can be a bit tricky. Give it a go and let me know if you need help. Overall what I'd try to do is commit the upstream changes[1] on top of our debian/sid branch, build/test using the Debian tooling and once things are working rebase onto the plain upstream branch then push that as a PR. The problem you're going to run into is dpkg-source complaining about unrepresented upstream changes. IIRC you can bypass that problem by only doing binary builds (check dpkg-buildpackage.1 for how to do that) or if all else fails temporarily switching the package to be format=3.0 (native) instead of (quilt). [1]: Dealing with changes that need both upstream and Debian adjustments is probably where git-debrebase would come in handy as it (IIRC) can seperate those a mixed upstream+Debian commit into seperate ones and then you can easily get rid of the Debian specific stuff during the final rebase. You can also just side-step that problem by committing such things separately to begin with. Re-ordering is easy whenever commits are non-overlapping after all. > > > Dne 28.05.2024 (tor) ob 19:23 +0200 je Daniel Gröber napisal(a): > > > > I'm not super happy with the approach of putting git-subrepo.d inside > > > > /usr/share/git-subrepo tbh. I might be able to let it pass but it seems > > > > lintian found another issue that needs patching anyway so you may as > > > > well > > > > fix this too. > > > > I'm still not seeing a change to remove git-subrepo.d. My comments don't > > just go away by ignoring them :P > > > I am sorry that you felt ignored. I simply thought i have a choice here and > tbh > i still do not see what could be wrong by having sourced only scripts in a > separate subdirectory. I'd be happy to understand your git-subrepo.d concerns > better to be able to fully support this decision. No worries. You do have a choice but you should at least communicate why you don't want to do what I suggest i.e. all review comments should be responded to with something lik ACK or NACK+further-discussion. It's not so much about "something wrong" as it is just a matter of having tidy and predictable structure. > > I noticed another thing, we disable the test suite for what appear to > > be trivial reasons. I really don't like maintaining packages without a > > test suite so this is a no-go. Please look into why it's not working > > and fix it with more upstreamable patches if
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Hi Daniel, Thanks for the review. I'll do my best to include as much as i know how to into another 'release candidate', however it may take me a while. I also have some additional concerns/questions below... Dne 10.06.2024 (pon) ob 22:26 +0200 je Daniel Gröber napisal(a): > In general you're missing the Debian patch metadata, see [DEP-3]. I hate > this archaic stuff I just mention it for completness. If you use gbp-pq for > generating the patches you can mostly ignore. I do like to use the > Forwarded field to note the upstream PR for the patch tho. > > [DEP-3]: https://dep-team.pages.debian.net/deps/dep3/ > > Next time I see the patches I want to see a Forwarded field for each -- one > PR for all of them pleas, not spam upstream :) > I am a bit confused. I'm not sure i want to present patches upstream while you (at least) have not yet accepted them. Otherwise i'd have to revise my upstream proposal (and spam upstream) every time you find another thing i missed about the debian policy (which i probably will for some time). How do you see this? > > Dne 28.05.2024 (tor) ob 19:23 +0200 je Daniel Gröber napisal(a): > > > I'm not super happy with the approach of putting git-subrepo.d inside > > > /usr/share/git-subrepo tbh. I might be able to let it pass but it seems > > > lintian found another issue that needs patching anyway so you may as well > > > fix this too. > > I'm still not seeing a change to remove git-subrepo.d. My comments don't > just go away by ignoring them :P > I am sorry that you felt ignored. I simply thought i have a choice here and tbh i still do not see what could be wrong by having sourced only scripts in a separate subdirectory. I'd be happy to understand your git-subrepo.d concerns better to be able to fully support this decision. > I noticed another thing, w > e disable the test suite for what appear to be > trivial reasons. I really don't like maintaining packages without a test > suite so this is a no-go. Please look into why it's not working and fix it > with more upstreamable patches if necessary. > > From a quick look it seems removing the `git config core.autocrlf input` > call in test/setup already gets us quite far but the way git subrepo finds > its libs needs adjustment too. > > Commit review below: > > > From: Samo Pogačnik > > > > Default 'git-core' location of git-subrepo executables currently > > +The default 'git-core' ... ? thanks > > > does not automatically integrate git-subrepo into git's own bash- > > completion. This change moves git-subrepo executables into > > /usr/share/git-subrepo and adds a symlink of its main executable > > script into /usr/bin to achieve recognition of the 'git subrepo' > > sub-command under the git bash-completion (through git's: > > --list-cmds=...,other,...). > > > > Gbp-Pq: Name 0001-Fixed-bash-completion-integration-with-git.patch > > --- > > Makefile | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/Makefile b/Makefile > > index 79898f5..3d84454 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -17,7 +17,7 @@ SHARE = share > > > > # Install variables: > > PREFIX ?= /usr/local > > -INSTALL_LIB ?= $(DESTDIR)$(shell git --exec-path) > > +INSTALL_LIB ?= $(DESTDIR)$(PREFIX)/share/$(NAME) > > While we're fixing upstream's mess $(DESTDIR) should be interpolated in the > install target only so overriding the directory structure is easier. > > The install target should look something like this, vars listed for > clarity: > > ``` > PREFIX ?= /usr/local > INSTALL_LIB ?= $(PREFIX)/share/$(NAME) > install: > $(INSTALL) -d -m 0755 $(DESTDIR)$(INSTALL_LIB)/ > $(INSTALL) -C -m 0755 $(LIB) $(DESTDIR)$(INSTALL_LIB)/ > ``` > > Notice the canonical use of $(INSTALL) instead of the plain command, > doesn't make a difference in this case but it's good Makefile hygiene. > > With that setup we can just export the INSTALL_* vars in debian/rules to > override them: > > export INSTALL_LIB=/usr/share/git-subrepo > export INSTALL_EXT=$(INSTALL_LIB) > > Setting INSTALL_EXT equal to INSTALL_LIB gets rid of the git-subrepo.d as > I've been requesting. > > I'm sending you the full patch "Drop unecessary subdir in usr/share" I used > to test this seperately, lets see if you can figure out how to apply it to > your repo ;) > > You still have to add a seperate commit to make the $(DESTDIR) adjustment. > > > INSTALL_EXT ?= $(INSTALL_LIB)/$(NAME).d > > INSTALL_MAN1 ?= $(DESTDIR)$(PREFIX)/share/man/man1 > > > > @@ -67,6 +67,8 @@ install: > > install -C -m 0755 $(EXTS) $(INSTALL_EXT)/ > > install -d -m 0755 $(INSTALL_MAN1)/ > > install -C -m 0644 $(MAN1)/$(NAME).1 $(INSTALL_MAN1)/ > > + install -d -m 0755 $(DESTDIR)$(PREFIX)/bin/ > > + ln -s ../share/$(NAME)/$(NAME) $(DESTDIR)$(PREFIX)/bin/$(NAME) > > Creating symlinks like this we'd usually do with dh_link(1). This would be > OK if you're intending to send this patch upstream? > > > > > # Uninstall support: >
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Hi Samo, On Sat, Jun 01, 2024 at 10:01:53AM +0200, Samo Pogačnik wrote: > I prepared a new 0.4.6-1 release with patched upstream regarding: > - bash-completition integration with git > - executable permissions on sourced-only files > - hashbangs inside sourced-only files > > I've also removed old lintian-override from debian/, since it is not > required anymore. Thanks, commit review inline below. In general you're missing the Debian patch metadata, see [DEP-3]. I hate this archaic stuff I just mention it for completness. If you use gbp-pq for generating the patches you can mostly ignore. I do like to use the Forwarded field to note the upstream PR for the patch tho. [DEP-3]: https://dep-team.pages.debian.net/deps/dep3/ Next time I see the patches I want to see a Forwarded field for each -- one PR for all of them pleas, not spam upstream :) > Dne 28.05.2024 (tor) ob 19:23 +0200 je Daniel Gröber napisal(a): > > I'm not super happy with the approach of putting git-subrepo.d inside > > /usr/share/git-subrepo tbh. I might be able to let it pass but it seems > > lintian found another issue that needs patching anyway so you may as well > > fix this too. I'm still not seeing a change to remove git-subrepo.d. My comments don't just go away by ignoring them :P I noticed another thing, we disable the test suite for what appear to be trivial reasons. I really don't like maintaining packages without a test suite so this is a no-go. Please look into why it's not working and fix it with more upstreamable patches if necessary. From a quick look it seems removing the `git config core.autocrlf input` call in test/setup already gets us quite far but the way git subrepo finds its libs needs adjustment too. Commit review below: > From: Samo Pogačnik > > Default 'git-core' location of git-subrepo executables currently +The default 'git-core' ... ? > does not automatically integrate git-subrepo into git's own bash- > completion. This change moves git-subrepo executables into > /usr/share/git-subrepo and adds a symlink of its main executable > script into /usr/bin to achieve recognition of the 'git subrepo' > sub-command under the git bash-completion (through git's: > --list-cmds=...,other,...). > > Gbp-Pq: Name 0001-Fixed-bash-completion-integration-with-git.patch > --- > Makefile | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 79898f5..3d84454 100644 > --- a/Makefile > +++ b/Makefile > @@ -17,7 +17,7 @@ SHARE = share > > # Install variables: > PREFIX ?= /usr/local > -INSTALL_LIB ?= $(DESTDIR)$(shell git --exec-path) > +INSTALL_LIB ?= $(DESTDIR)$(PREFIX)/share/$(NAME) While we're fixing upstream's mess $(DESTDIR) should be interpolated in the install target only so overriding the directory structure is easier. The install target should look something like this, vars listed for clarity: ``` PREFIX ?= /usr/local INSTALL_LIB ?= $(PREFIX)/share/$(NAME) install: $(INSTALL) -d -m 0755 $(DESTDIR)$(INSTALL_LIB)/ $(INSTALL) -C -m 0755 $(LIB) $(DESTDIR)$(INSTALL_LIB)/ ``` Notice the canonical use of $(INSTALL) instead of the plain command, doesn't make a difference in this case but it's good Makefile hygiene. With that setup we can just export the INSTALL_* vars in debian/rules to override them: export INSTALL_LIB=/usr/share/git-subrepo export INSTALL_EXT=$(INSTALL_LIB) Setting INSTALL_EXT equal to INSTALL_LIB gets rid of the git-subrepo.d as I've been requesting. I'm sending you the full patch "Drop unecessary subdir in usr/share" I used to test this seperately, lets see if you can figure out how to apply it to your repo ;) You still have to add a seperate commit to make the $(DESTDIR) adjustment. > INSTALL_EXT ?= $(INSTALL_LIB)/$(NAME).d > INSTALL_MAN1 ?= $(DESTDIR)$(PREFIX)/share/man/man1 > > @@ -67,6 +67,8 @@ install: > install -C -m 0755 $(EXTS) $(INSTALL_EXT)/ > install -d -m 0755 $(INSTALL_MAN1)/ > install -C -m 0644 $(MAN1)/$(NAME).1 $(INSTALL_MAN1)/ > + install -d -m 0755 $(DESTDIR)$(PREFIX)/bin/ > + ln -s ../share/$(NAME)/$(NAME) $(DESTDIR)$(PREFIX)/bin/$(NAME) Creating symlinks like this we'd usually do with dh_link(1). This would be OK if you're intending to send this patch upstream? > > # Uninstall support: > uninstall: > -- > 2.39.2 > > From: Samo Pogačnik > > Bash scripts under git-suberpo.d are not to be executed standalone > therefore should not have executable permissions. > > Gbp-Pq: Name 0002-Removed-executable-permission-on-sourced-only-files.patch > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 3d84454..818cd7d 100644 > --- a/Makefile > +++ b/Makefile > @@ -64,7 +64,7 @@ install: > install -C -m 0755 $(LIB) $(INSTALL_LIB)/ > sed -i 's!^SUBREPO_EXT_DIR=.*!SUBREPO_EXT_DIR=$(INSTALL_EXT)!' > $(INSTALL_LIB)/$(NAME) > install -d -m 0755 $(INSTALL_EXT)/ > - install -C -m
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Hi Daniel, Dne 28.05.2024 (tor) ob 19:23 +0200 je Daniel Gröber napisal(a): > I'm not super happy with the approach of putting git-subrepo.d inside > /usr/share/git-subrepo tbh. I might be able to let it pass but it seems > lintian found another issue that needs patching anyway so you may as well > fix this too. > > W: git-subrepo: bash-completion-with-hashbang bash [usr/share/bash- > completion/completions/git-subrepo:1] > W: git-subrepo: executable-not-elf-or-script [usr/share/git-subrepo/git- > subrepo.d/bash+.bash] > W: git-subrepo: mismatched-override executable-not-elf-or-script > usr/lib/git-core/git-subrepo.d/bash+.bash [usr/share/lintian/overrides/git- > subrepo:1] > N: 0 hints overridden; 1 unused override > > indeed bash+.bash is +x but shouldn't be. > > I'm not sure whether bash-completion-with-hashbang should really be W > severity but the '#!bash' it has is certainly completely wrong. Looks like > you'll have to get over your fear of patching upstream ;) > I prepared a new 0.4.6-1 release ( https://salsa.debian.org/spog/git-subrepo/-/commit/ebbc6e9df46b479f1517a5dbd8486b95f511d5be ) with patched upstream regarding: - bash-completition integration with git - executable permissions on sourced-only files - hashbangs inside sourced-only files I've also removed old lintian-override from debian/, since it is not required anymore. --Samo
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Hi Samo, On Sat, May 25, 2024 at 07:30:46PM +0200, Samo Pogačnik wrote: > https://salsa.debian.org/spog/git-subrepo/-/commit/42628d43faa4a05eb3dd3c4b75d9d194ce6bda90 I'm not super happy with the approach of putting git-subrepo.d inside /usr/share/git-subrepo tbh. I might be able to let it pass but it seems lintian found another issue that needs patching anyway so you may as well fix this too. W: git-subrepo: bash-completion-with-hashbang bash [usr/share/bash-completion/completions/git-subrepo:1] W: git-subrepo: executable-not-elf-or-script [usr/share/git-subrepo/git-subrepo.d/bash+.bash] W: git-subrepo: mismatched-override executable-not-elf-or-script usr/lib/git-core/git-subrepo.d/bash+.bash [usr/share/lintian/overrides/git-subrepo:1] N: 0 hints overridden; 1 unused override indeed bash+.bash is +x but shouldn't be. I'm not sure whether bash-completion-with-hashbang should really be W severity but the '#!bash' it has is certainly completely wrong. Looks like you'll have to get over your fear of patching upstream ;) --Daniel signature.asc Description: PGP signature
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Dne 06.05.2024 (pon) ob 15:02 +0200 je Daniel Gröber napisal(a): > > Hmm. I'm not sure I like the idea of abusing libexec in this > way. Technically speaking it's for "internal binaries that are not intended > to be executed directly by users or shell scripts" this is clearly not the > case here. > > Looking over [git-* packages] to see what other packages do I see most git > addons are in fact installed into /usr/bin. > > [git-* packages]: > https://packages.debian.org/search?searchon=contents=git-=filename=stable=any > > Notable exception: git-absorb is still in lib/git-core and has [Bug#985775] > which seems like it may be relevant to our problem. Have a read of it when > you get the chance. Some snippets "Git changed the gitexecdir directory" > maybe that change is what broke completion from git-core? "... git-absorb > works but git's bash completion doesn't suggest absorb" So they are aware > of the completion issue already. > > [Bug#985775]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=985775 I also believe, this bug indicates the same problem. Unfortunately nothing have changed since 2021. A reference to a git change in the bug report indicates that one can add completion commands like: git config --global completion.commands subrepo I've tested that and it works, however i am not a fan of this solution, while automatic detection of /usr/bin/git-* is available. > > Anyway. I think the right solution here is to patch git-subrepo. IMO what's > in git-subrepo.d really belongs in /usr/share/git-subrepo OR could just be > concatenated into the main git-subrepo script, hmm. Bonus points for > forwarding this bug and patch upstream. > I prepared another change regarding git bash-completion integration using "/usr/bin" and "/usr/share" in a similar way as some other packages (i.e. dcut, dput, lintian, ...). And yes it may be worth asking upstream how do they feel about the problem. For now i've avoided touching upstream code, but if you prefer i can try to rework the change in upstream code and produce first patch for the package. --Samo
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Hi Daniel, Dne 06.05.2024 (pon) ob 15:02 +0200 je Daniel Gröber napisal(a): > > To be clear force-push should never ever be done when collaborating on the > branche(s) with multiple people, except in the most dire of circumstances > and only if everyone involved is notified appropriately. I'll be happy to > give you commit rights on the repo as soon as you show you've internalised > this :) > I am aware that force-push on shared branches causes a mess for all other branch users. For that reason i'd do initial work on my forked repo to propose changes and use force-push exclusively on my fork to reset its state to what is already in the collab-repo as needed. > I thought we agreed on using plain gbp for now? > Exactly, i temporarily used my old dgitized fork, until a new collab-maint-repo is ready. I am sorry, if i did not explain mysef enough regarding that, while working on bash-completion issue. Now my dgitized repo is unforked and renamed to git-subrepo_dgit and a new fork is created: https://salsa.debian.org/spog/git-subrepo Then i also prepared a fresh update: git clone g...@salsa.debian.org:spog/git-subrepo.git cd git-subrepo/ git remote -v git remote add upstream https://github.com/ingydotnet/git-subrepo.git git remote -v git fetch upstream gbp import-ref -u 0.4.6 gbp dch --snapshot --auto debian/ vim debian/changelog git diff gbp buildpackage --git-ignore-new gbp dch --release --auto git diff git commit -m"Release 0.4.6-1" debian/changelog gbp buildpackage git push git switch upstream git pull upstream master git log git reset --hard HEAD~1 git log git push --tags origin debian/sid upstream git switch debian/sid This is now the updated state pushed into my fork without additional changes regarding bash-completion. > > 73a01 | | * upstream origin/upstream docs: Replace 404$ Edwin Kofler 5M > | |/ > 110b9 | * 0.4.6 Release 0.4.6Austin Morgan 1Y > > The upstream branch is ahead of the 0.4.6 tag. Why? Seems to me you meddled > with the upstream branch by hand instead of letting the tooling take care > of it. Technicaly not a problem just makes me wonder what you're doing. > I wonder why the tool didn't care about it again (see above). Perhaps because the upstream branch has not been checked out initially (oh, i should've probably called gbp clone instead of git clone - my bad). I'll prepare another change regarding bash-completion later. I saw a few examples using "/usr/share" from "/usr/bin" (i.e. dcut, dput, lintian, ...). Anyway, thanks for the thorough review, --Samo
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Hi Samo, On Sat, Apr 27, 2024 at 10:48:01AM +0200, Samo Pogačnik wrote: > > So you should be doing something like this: > > > > $ git remote add upstream https://github.com/ingydotnet/git-subrepo.git > > $ git fetch upstream > > $ gbp import-ref -u 0.4.6 # this will do the upstream tag/branch > > # changes and create the merge > > $ gbp dch > > > > $ gbp buildpackage [...sbuild runes...] > > > > $ git push --tags salsa debian/sid upstream > > > > There's also `gbp push` to make the git-push easier but it only works after > > doing a `gbp tag` to create the debian/* tag. This is however inappropriate > > for you as DM to do as the convention is to have the debian tag correspond > > to what I upload not what you propose to me :) > > > > On my side I'll do: > > > > $ gbp pull samo > > > > $ gbp buildpackage [...sbuild runes...] > > > > $ gbp tag# creates the debian/* tag > > $ debrelease # upload to ftp-master > > $ gbp push salsa > > > > Hope that gives you something more actionable than my previous mails. > > Thanks for this explanation. I suppose we shall apply this workflow upon > the git-repo, that you are going to push into the debian/* namespace. Yup, just want to make sure you have the ins and outs of the workflow down first. > As i understand, we are going to push and pull our changes from the same > branches of the sam git-repo, however i got a bit confused by your 'gbp > pull samo' command which indicates another git-repo involved. If your > initial command should've been 'gbp pull salsa', then it is clear to me. There's not much difference really. I'm just a stickler for clean git history and also want to make sure new contributors un-learn the really bad force-pushing habbit Git-hub/lab/tea teach people. To be clear force-push should never ever be done when collaborating on the branche(s) with multiple people, except in the most dire of circumstances and only if everyone involved is notified appropriately. I'll be happy to give you commit rights on the repo as soon as you show you've internalised this :) On Wed, May 01, 2024 at 11:09:48PM +0200, Samo Pogačnik wrote: > i noticed, that bash-completion integration with git does not work. Good catch, I don't use completions at that level much myself so I probably wouldn't have noticed :) > However i noticed, that the --list-cmds group 'other' scans the /usr/bin/ > directory, where our old friend 'git-debrebase' and friends already > reside and are being successfully recognized by git. Thus i prepared a > change (in my salsa repo: in > https://salsa.debian.org/spog/git-subrepo/-/tree/debian/sid) to change > the target installation directory, which seems to work. Sounds like a reasonable plan. On Sat, May 04, 2024 at 01:48:56PM +0200, Samo Pogačnik wrote: > Unfortunately, lintian is not happy with my solution above (does not > allow subdirs in /usr/bin and however git-subrepo script searches its > helper functions in git-subrepo.d subdir in the location of main > git-subrepo script). Right, that doesn't comply with policy at all. My first instinct would be to patch git-subrepo to parameterise the path to git-subrepo.d and send that patch upstream. > I managed to prepare another solution, without changing upstream sources > in a way to move 'git-subrepo' executable scripts into > /usr/libexec/git-subrepo and by adding a symlink to main git-subrepo > executable into /usr/bin. That way bash- completion integration works as > in the initial solution and lintian is also happy. Hmm. I'm not sure I like the idea of abusing libexec in this way. Technically speaking it's for "internal binaries that are not intended to be executed directly by users or shell scripts" this is clearly not the case here. Looking over [git-* packages] to see what other packages do I see most git addons are in fact installed into /usr/bin. [git-* packages]: https://packages.debian.org/search?searchon=contents=git-=filename=stable=any Notable exception: git-absorb is still in lib/git-core and has [Bug#985775] which seems like it may be relevant to our problem. Have a read of it when you get the chance. Some snippets "Git changed the gitexecdir directory" maybe that change is what broke completion from git-core? "... git-absorb works but git's bash completion doesn't suggest absorb" So they are aware of the completion issue already. [Bug#985775]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=985775 Anyway. I think the right solution here is to patch git-subrepo. IMO what's in git-subrepo.d really belongs in /usr/share/git-subrepo OR could just be concatenated into the main git-subrepo script, hmm. Bonus points for forwarding this bug and patch upstream. Git Review -- Now, let's get into the review. Here's what I see -- if you're not reading this in a monospace font this is the time to reconsider your ~life~ eer. config choices :D 84c24 *
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Hi Samo, thanks for the quick review! On Mon, Jan 04, 2021 at 09:01:51PM +0100, Samo Pogačnik wrote: > Thanks for that. I sincerely hope, that you get through with this useful tool. I saw you submitted an RFS for git-subrepo a while back. I was wondering what happened with that. It's not clear from what's in the BTS why it didn't get uploaded, did you just not get a response from anyone? In case you are still interested in maintaining git-subrepo perhaps we could co-maintian it if you like? > By the way, your salsa (Vcs:) link does not work (guest-dxld should > probably be dxld-guest). Ah good catch, fixed in 0.4.3-2. Side note: I'm not sure if I'm supposed to increment the debian revision during the mentors process or if should I just keep the initial -1 revision. Thanks, --Daniel PS: You seem to have replied off-list, do you mind if I forward my response to the BTS/debian-mentors list also?
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Hi Samo, On Mon, Jan 04, 2021 at 11:16:04PM +0100, Samo Pogačnik wrote: > Dne 04.01.2021 (pon) ob 22:11 +0100 je Daniel Gröber napisal(a): > > I saw you submitted an RFS for git-subrepo a while back. I was wondering > > what happened with that. It's not clear from what's in the BTS why it > > didn't get uploaded, did you just not get a response from anyone? > there was simply no response to my RFS and 'mentors' upload got automatically > deleted about a month ago (after almost a year). Ah, I see. I didn't know mentors.d.net garbage collects packages automatically. > I have no extra preferences for maintaining the package because i am very > inexperienced regarding debian packaging. Otherwise i am willing to help... It's up to you, if you don't feel up to it I'm happy to do it. It's just always nice to have someone else that can jump in with package updates. Bus-factor and all. > Maybe i could not get more attention because of the 'native' type of my > package which it self uses 'git subrepo' instead of the preferred 'quilt' > approach (i really do not like managing series of patches, ...). I would hope people would have simply complained about that if it was in fact the problem. Seems to me there just aren't that many DDs actively watching the mentors list or maybe people do reviews mostly off-list? Had I seen the RFS I would have definitely complained about the weird repo structure in the review ;) I think people in Debian are trying to move to git-buildpackage (see also DEP-14[1]) as the main way to manage packageing in git, and since it comes with it's own way of importing upstream releases and such using git-subrepo for the packaging is unlikely to go over well. [1]: https://dep-team.pages.debian.net/deps/dep14/ I plan on using gbp-pq to manage patches as regular git commits. That seems reasonably more comfortable than plain quilt, which I agree is a PITA. > I even had the idea that git-subrepo is an excelent tool to enhance and > maybe simplify debian packaging using git, but again i am too > inexperienced to provide valid proposals covering all aspects of debian > packaging. Honestly I don't think it's worth swimming against the gbp stream at this point. Personally I'd rather have one(ish) way of doing Debian packaging in git so I can just jump into any package with debcheckout and be immediately productive, instead of having to learn each maintainer's weird conventions and tools fist. One can always improve gbp if it doesn't optimally cover a particular workflow yet. > You are of course welcome to inspect my github project: > https://github.com/spog/git-subrepo-debian I had a look of course when I started packaging, but decided to start fresh with gbp since I prefer forking the debian packaging from the upstream git repo as it makes inspecting the history and git-bisect'ing across upstream and debian revisions more streamlined. --Daniel
Bug#979188: RFS: git-subrepo/0.4.3-1 [ITP] -- Alternative to git-submodule(1) and git-subtree(1)
Package: sponsorship-requests Severity: wishlist Dear mentors, I am looking for a sponsor for my package "git-subrepo": * Package name: git-subrepo Version : 0.4.3-1 Upstream Author : Ingy döt Net * URL : https://github.com/ingydotnet/git-subrepo * License : GPL-2+ and GPL-2, MIT * Vcs : https://salsa.debian.org/guest-dxld/git-subrepo Section : vcs git-subrepo allows embedding and managing copies of other git repositories in your repo for purposes such as vendoring. Commits in the parent repo involving the subrepo can easily be pushed back upstream even when they touch files outside the subrepo -- these will simply be omitted. Pulling new upstream commits back into your repo is naturally also possible. git-subrepo is designed such that only the maintainer of a repo will actually need to have it installed, contributors only need to do so if they wish to push/pull from the subrepo's upstream and user should never have to interact with git-subrepo at all since all of a subrepos file are available right after a plain git-clone. This is unlike git-submodule(1)s where all users and contributors must be aware of their presence and deal with them. git-subrepo is in principle somewhat similar to git-subtree(1) as it also embedds snapshot copies of other repos, however it is much easier to use and the way the history is kept is less convoluted. Pulling any number of new commits from a subrepo's upstream will result in only a single commit in the parent repo for example. It builds this binary package: git-subrepo - Alternative to git-submodule(1) and git-subtree(1) To access further information about this package, please visit the following URL: https://mentors.debian.net/package/git-subrepo/ Alternatively, one can download the package with dget using this command: dget -x https://mentors.debian.net/debian/pool/main/g/git-subrepo/git-subrepo_0.4.3-1.dsc Changes for the initial release: git-subrepo (0.4.3-1) unstable; urgency=medium . * Initial release. (Closes: #911397) Regards, Daniel signature.asc Description: PGP signature
