| Message ID | 120d2bb3e461717e5248bb4c97feab86d4e45c9d.1597655273.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Optionally skip linking/copying the built-ins | expand |
Hi Peff, On Mon, 17 Aug 2020, Jeff King wrote: > On Mon, Aug 17, 2020 at 09:07:51AM +0000, Johannes Schindelin wrote: > > > - $(INSTALL) git.pdb '$(DESTDIR_SQ)$(bindir_SQ)' > > - $(INSTALL) git-shell.pdb '$(DESTDIR_SQ)$(bindir_SQ)' > > - $(INSTALL) git-upload-pack.pdb '$(DESTDIR_SQ)$(bindir_SQ)' > > - $(INSTALL) git-credential-store.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > - $(INSTALL) git-daemon.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > - $(INSTALL) git-fast-import.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > - $(INSTALL) git-http-backend.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > - $(INSTALL) git-http-fetch.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > - $(INSTALL) git-http-push.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > - $(INSTALL) git-imap-send.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > - $(INSTALL) git-remote-http.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > - $(INSTALL) git-remote-testsvn.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > - $(INSTALL) git-sh-i18n--envsubst.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > - $(INSTALL) git-show-index.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > + $(INSTALL) $(patsubst %.exe,%.pdb,$(filter-out $(BUILT_INS),$(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)))) '$(DESTDIR_SQ)$(bindir_SQ)' > > + $(INSTALL) $(patsubst %.exe,%.pdb,$(filter-out $(BUILT_INS) $(REMOTE_CURL_ALIASES),$(PROGRAMS))) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > Oh, this is much better than what my patch does. :) > > The rest of the series looks like a good direction to me, but is outside > the scope of my series. I'd be happy to pick this first patch up for a > re-roll of mine (which would require tweaking the rest of the patches on > top to stop removing things from the .pdb list). Or we could just leave > this as a separate topic and deal with the merge conflict (which would > obviously resolve in favor of yours). Please feel totally free to cherry-pick my 1/3 as your 1/5 (but please do fix up the author email address, if you don't mind). I have no problem with my patch series depending on yours, to make merging easier. Ciao, Dscho
On Mon, Aug 17, 2020 at 09:07:51AM +0000, Johannes Schindelin wrote: > - $(INSTALL) git.pdb '$(DESTDIR_SQ)$(bindir_SQ)' > - $(INSTALL) git-shell.pdb '$(DESTDIR_SQ)$(bindir_SQ)' > - $(INSTALL) git-upload-pack.pdb '$(DESTDIR_SQ)$(bindir_SQ)' > - $(INSTALL) git-credential-store.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > - $(INSTALL) git-daemon.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > - $(INSTALL) git-fast-import.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > - $(INSTALL) git-http-backend.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > - $(INSTALL) git-http-fetch.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > - $(INSTALL) git-http-push.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > - $(INSTALL) git-imap-send.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > - $(INSTALL) git-remote-http.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > - $(INSTALL) git-remote-testsvn.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > - $(INSTALL) git-sh-i18n--envsubst.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > - $(INSTALL) git-show-index.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > + $(INSTALL) $(patsubst %.exe,%.pdb,$(filter-out $(BUILT_INS),$(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)))) '$(DESTDIR_SQ)$(bindir_SQ)' > + $(INSTALL) $(patsubst %.exe,%.pdb,$(filter-out $(BUILT_INS) $(REMOTE_CURL_ALIASES),$(PROGRAMS))) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' Oh, this is much better than what my patch does. :) The rest of the series looks like a good direction to me, but is outside the scope of my series. I'd be happy to pick this first patch up for a re-roll of mine (which would require tweaking the rest of the patches on top to stop removing things from the .pdb list). Or we could just leave this as a separate topic and deal with the merge conflict (which would obviously resolve in favor of yours). -Peff
On Mon, Aug 17, 2020 at 07:51:17AM +0200, Johannes Schindelin wrote: > > The rest of the series looks like a good direction to me, but is outside > > the scope of my series. I'd be happy to pick this first patch up for a > > re-roll of mine (which would require tweaking the rest of the patches on > > top to stop removing things from the .pdb list). Or we could just leave > > this as a separate topic and deal with the merge conflict (which would > > obviously resolve in favor of yours). > > Please feel totally free to cherry-pick my 1/3 as your 1/5 (but please do > fix up the author email address, if you don't mind). > > I have no problem with my patch series depending on yours, to make merging > easier. It doesn't look like that series otherwise needs a re-roll, so if it's OK with you, let's just have yours come on top (or independent, as the conflict is pretty easy). -Peff
Hi Peff, On Mon, 17 Aug 2020, Jeff King wrote: > On Mon, Aug 17, 2020 at 07:51:17AM +0200, Johannes Schindelin wrote: > > > > The rest of the series looks like a good direction to me, but is outside > > > the scope of my series. I'd be happy to pick this first patch up for a > > > re-roll of mine (which would require tweaking the rest of the patches on > > > top to stop removing things from the .pdb list). Or we could just leave > > > this as a separate topic and deal with the merge conflict (which would > > > obviously resolve in favor of yours). > > > > Please feel totally free to cherry-pick my 1/3 as your 1/5 (but please do > > fix up the author email address, if you don't mind). > > > > I have no problem with my patch series depending on yours, to make merging > > easier. > > It doesn't look like that series otherwise needs a re-roll, so if it's > OK with you, let's just have yours come on top (or independent, as the > conflict is pretty easy). Sounds good to me! Ciao, Dscho
diff --git a/Makefile b/Makefile index 372139f1f2..3f5ba97b70 100644 --- a/Makefile +++ b/Makefile @@ -2899,20 +2899,8 @@ ifdef MSVC # have already been rolled up into the exe's pdb file. # We DO NOT have pdb files for the builtin commands (like git-status.exe) # because it is just a copy/hardlink of git.exe, rather than a unique binary. - $(INSTALL) git.pdb '$(DESTDIR_SQ)$(bindir_SQ)' - $(INSTALL) git-shell.pdb '$(DESTDIR_SQ)$(bindir_SQ)' - $(INSTALL) git-upload-pack.pdb '$(DESTDIR_SQ)$(bindir_SQ)' - $(INSTALL) git-credential-store.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-daemon.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-fast-import.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-http-backend.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-http-fetch.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-http-push.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-imap-send.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-remote-http.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-remote-testsvn.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-sh-i18n--envsubst.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-show-index.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' + $(INSTALL) $(patsubst %.exe,%.pdb,$(filter-out $(BUILT_INS),$(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)))) '$(DESTDIR_SQ)$(bindir_SQ)' + $(INSTALL) $(patsubst %.exe,%.pdb,$(filter-out $(BUILT_INS) $(REMOTE_CURL_ALIASES),$(PROGRAMS))) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' ifndef DEBUG $(INSTALL) $(vcpkg_rel_bin)/*.dll '$(DESTDIR_SQ)$(bindir_SQ)' $(INSTALL) $(vcpkg_rel_bin)/*.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
There is a hard-coded list of `.pdb` files to copy. But we are about to introduce the `SKIP_DASHED_BUILT_INS` knob in the `Makefile`, which might make this hard-coded list incorrect. Let's switch to a dynamically-generated list instead. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- Makefile | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-)