I don't think this is the correct fix; just because I run make install doesn't mean I want the debug version removed from the destination directory; that makes it difficult to keep the debug version around when I'm building/updating just the release version.

Rather, we should figure out why the debug library is getting built/installed when we don't want it. Most likely there's something wrong with #27814 and libraries are getting copied into the destdir when they shouldn't be.

I certainly did not mean to close this PR. Sorry about that. :)

@staticfloat thanks for the review and comments.

I'm confused by your review, this is only for binary-dist .

I'm confused by your review, this is only for binary-dist .

It's not only for binary-dist; these changes are within the install: target, so they will apply whenever I run make install. Here's why I'm confused:

If I run make install without BUNDLE_DEBUG_LIBS, the debug libraries should not get installed. Which I believe is true.

If I run make binary-dist, the make system should clean out any previously-run make binary-dist objects (which is part of the distclean target), so the removal that you are doing in this PR should be having no effect anyway. So in what situation is make binary-dist copying in debug libraries?

this removes it from the bindir directory not the install directory. I've tested this locally and it fixes the bug in the linked issue by ensuring the debug julia lib is not included in the julia installer on windows.

I believe the offending line is

We should remedy the real issue, rather than just deleting the libjulia-debug.dll. The rule above should just be made specific to copy only the necessary files. Should hopefully not be too difficult. The Mac lines just below suggest that it should be easy to do so.

Indeed @ViralBShah that is the exact crux of the problem.

I decided to rm the file in this PR as it seemed the simplest approach since there numerous library files that need to be copied over in the catchall *.dll .

What's then the best way to specify to copy over all library dlls except libjulia-debug.dll ? We certainly don't want to specify all 36 dll files manually.

It seems part of the issue with using JL_PRIVATE_LIBS is it doesn't include all the libraries needed by windows.

Any thoughts on the comments above? Is this not sufficient (it does indeed work) until a better solution is proposed?

this removes it from the bindir directory not the install directory.

I'm not sure what you mean by this; $(DESTDIR)$(bindir) is the install directory, and we should not be deleting things from there. Imagine what happens when a user runs sudo make install prefix=/usr/local; we do not ever want to be deleting things from $(DESTDIR)$(bindir), assuming that we control what is within there, because we do not. We control $(build_bindir) but not $(DESTDIR)$(bindir).

Yes, we need to prevent libjulia-debug.dll from being copied over, since deletion is dangerous for the reason @staticfloat mentions.

@staticfloat, if we clearly remove the exact filename that we just copied over, is it ok? This only applies to Windows inside the ifdef, and will only be used by the buildbots (realistically). Perhaps the risk is contained enough to do the hack.

and will only be used by the buildbots.

I'm not sure why this is true. What limits this to only running on buildbots? If I am a user and I run make debug && make install BUNDLE_DEBUG_LIBS=1 prefix=/path, then later run make release && make install prefix=/path, what stops this branch from rendering my previously-installed debug installation useless?

I think if we just want to exclude libjulia-debug.dll we can do something like $(INSTALL_M) $(filter-out libjulia-debug.dll,$(wildcard $(build_bindir)/*.dll)) $(DESTDIR)$(bindir)/

thanks @staticfloat for the suggestion. Frustratingly it still ends up copying, is there something wrong with the filter-out and wildcard statement you posted? Looks correct, however.

You can debug this by doing things like echo "DOES THIS LOOK RIGHT: $(filter-out libjulia-debug.dll,$(wildcard $(build_bindir)/*.dll))" in the Makefile or whatnot. If you want to see things in a Makefile that are not getting run as part of a target, you can use $(warning blah blah blah) within top-level Makefiles statements.

Ok fixed! Thanks @staticfloat !

This also needs a backport label at least for 1.1 if not 1.0.x also.

Looks good to merge.