Re: [Libcdio-devel] Pete Batard's Changes (MinGW, MSVC, UDF, Joliet, header ...) merged in. Leon's CD-Text changes coming

[Pete Batard]

On 2012.03.09 02:09, Rocky Bernstein wrote:
> > We also modify the win32 cdrom driver files so that
> > 1. It works with Visual Studio
> > 2. It doesn't require end user applications to link with any additional
> > libraries beside the libcdio ones
>
> I don't understand why this is a win in all cases. As you say in the
> commit, this dynamically links with winmm.dll rather than linking against
> winmm.lib. Isn't there an advantage to not having to rely on finding
> winmm.dll at run time but instead have it statically linked into resulting
> binaries?

OK, the part you miss is that in both cases the dynamic library is used, 
even if winmm.lib is used (yeah, Windows is weird).
The .lib we are talking about here is an "import library", which is not 
a static version of the DLL, but instead an helper library that provides 
access the DLL at link time by letting applications know the function 
calls that are exported by the DLL.

In short, the process required libcdio app -> winmm.lib -> winmm.dll, 
with apps developer needing to specify a -lwinmm.lib equivalent for each 
libcdio app they compile, and this is the part I removed.

This can be a confusing aspect of MWVC in that a .lib can be a full 
fledged static lib or an import library that acts as an interface to a 
DLL. In all cases, considering that we only use 2 calls for winmm, we 
might as well remove the need to specify that library during compilation.

> Also, there is a little bit of code bloat. The headers inside
> MSVC/missing are fine, but the code inside win32 cdrom, especially
> lib/driver/MSWindows/win32.c is now more complicated.

I could factorize it if you think that's needed. For the record, the 
code we add is the code that happens behind the scenes when you link 
with the import lib, so the complexity is exactly the same. Only in one 
case this is done inside the import lib, and in the other we do it 
ourselves.

I think the benefit of having libcdio apps compiled with just -llibcdio 
rather than -llibcdio -lwinmm is worth the trouble.

> Given that you don't want to provide tests for the added complication, I
> hesitate to complicate.

Note that the code I added is something I have used and reused in many 
libraries and applications (libusb, libwdi, etc), and is something that 
is also commonly used by others, so it is very much tried and tested if 
that is your concern...

> > The one thing that may require some afterthought is the fact that we now
> > need to maintain a version.h in include/cdio/, since MSVC cannot
> > autogenerate one from the .in.
>
>
> Really? You can't run a program from MSVC that reads configure.ac and looks
> for: define(RELEASE_NUM, xx),  define (CDIO_VERSION_STR,  yy) , changes $1
> to xx in yy and then changes in  version.h.in @VERSION@ to xx and
> @address@hidden MSVC? If that is too much work, another suggestion is given
> below.

That's a discussion we've been having in libusb. Requires sed or some 
other program for which Windows does not have a native equivalent (yeah, 
Windows is natively that bad), so we would require every Windows 
developer to first go and download a win32 version of sed just to 
process 2 lines of data. This seems like a very inconvenient requirement 
to me.
Would you force all Linux developers wanting to recompile libcdio to go 
and download an extra utility if the situation was reversed?
The only acceptable way I see, outside of providing a sed.exe in the 
libcdio repo, is if we recompile and run our own simple search/replace 
parser in MSVC as a pre-build process, which can be done, but this adds 
to complexity and maintenance.

> I don't want to include version.h in include/cdio/version.h which can (and
> mostly will be) wrong.

The way I do it is, instead of having the maintainer manually editing 
the version of configure.ac, I have a chver.sh that I run from a shell 
script (MinGW or cygwin on Windows also do fine) that changes the 
version in all the relevant files at once, including version.h and 
configure.ac.

This is what I actually use in most of my projects, and I don't think 
one can say that editing a single file, instead of invoking a chver.sh 
script is much more preferable, since only the maintainer is supposed to 
change the version.

> Assuming it gets overwritten by autotools (except when it doesn't) leads
> to confusion and errors.

Do you have evidence that there exists cases where autotools will fail 
to overwrite an existing file from a .in?

I'm not currently aware of such an issue, and making versioning more 
complex (and _actually_ more prone to failure if it requires scripting 
on only one environment, MSVC, which is actually well known for breaking 
project files from one version to the next), just because we think there 
may exist a case where a file that should be overwritten isn't doesn't 
doesn't strike me as a convincing argument.
Unless you are aware of environment autotools fails to overwrite files, 
I think it is safe to assume that it will, especially as this is the 
expected behaviour.

> For MSVC, one could store a version of version.h in that MSVC directory and
> arrange the build system to move it to include/cdio/version.h before it is
> needed.

That can be done (with the caveat that there's no guarantee that Visual 
Studio 2012 will not break the move process we would use then), but I 
really only see that as necessary if we have actual reasons to believe 
that an existing version.h would not be overwritten in some cases. The 
simpler we can keep our versioning the better, and making it more 
complex to tackle an issue we have no evidence of being an actual 
problem doesn't strike me as a good investment.

> If this route is used rather than writing a program to create
> version.h, updating version.h should be the task of those who build on MSVC
> which is where the responsibility should lie.

Versioning is and always will be the task of the maintainer IMO. Else, 
you remove the whole point of versioning, which is that, if someone uses 
a library in the wild, which they compiled from official source, and you 
ask for a version, you get a value which you have knowledge of rather 
than a completely arbitrary one.
I don't see the versioning responsibility lying with anybody else than 
the maintainer of the source repo, which means that if someone uses an 
unmodified source repo, the maintainer of that original source is the 
one that should take care of it. It's only if someone creates a 
derivative that the responsibility shifts.

> And when that can't be done,
> I'd rather leave it to the last known verified value.

If your concern if having to maintain 2 files with a version instead of 
one, then I can provide you with a chver.sh script that updates both 
files at once.

> I looked too at this patch and it seems to spread MSVC ugliness around. Is
> it possible in a header file inside the MSVC directory LIBCDIO_DDL_DECLSPEC
> macro and the externs that need LIBCDIO_DLL_DECLSPEC? (And if this is done
> perhaps you don't even need the macro. Then arrange for this header to be
> included as the MSVC/config.h header is included?

The declspec has to be part of the public libcdio interface if you want 
to use the library as a DLL. So, unless we use a separate include 
directory for MSVC, we'd have to overwrite existing files before 
compilation or something. I don't know of a way to avoid the declspec 
import/export, if global variable access is required, that keeps the 
code compatible.
If you don't want the macros to appear for non Windows users, then 
really the 2 choices I see are:
- drop the DLL buildout on Windows
- maintain 2 independent sets of public headers (one for MSVC and one 
for other platforms).

Of course, the third option is to break the API and encapsulate these 
global libcdio variables, so that get/set methods are used instead of 
direct access. I actually think this would benefit libcdio, since direct 
access to library internal is something most libraries try to avoid, and 
most libraries I know seem to avoid that, but of course this would mean 
that existing applications will need a rewrite...

Also please note that what we are facing with libcdio here is nothing 
original. Every cross platform library that also wants to produce a 
Windows DLL has to make concessions in that respect, and as with the 
case with libcdio, whenever the library originates from POSIX, there is 
a lot of reluctance to pollute the headers with "MSVC ugliness". We went 
through pretty much the same for libusb (libusb didn't have global 
variables, but did require having to specify a calling convention 
through an extra MSVC specific prefix macro - and don't get me started 
on MinGW/MSVC DLL interchangeability), and it was quite acrimonious, 
which is a pity considering that the whole issue revolves around whether 
someone who looks at the library header should or should not see macros 
that don't do ANYTHING for any platform except Windows...

If you look at the libusb header [1], you will see that much of the 
functions are now prefixed with a LIBUSB_CALL, which is a direct result 
of making libusb available as a DLL on Windows. And you should see more 
examples of this if you look around (such as pthread-win32 [2], a port 
of pthreads for Windows, with PTW32_DLLPORT).
I'm afraid this is not something one can exactly avoid when trying to be 
cross platform compatible with a Windows DLL. Given that I've already 
been through this whole "but Windows DLL adds ugliness" business, I 
tried to minimize the impact as much as I could, but there's only so 
much that can be achieved, and POSIX has to do some concessions at one 
stage.

Therefore, I will state that the choice is simple really:
Either you don't want to spread "MSVC ugliness" around, and we drop the 
idea of providing a DLL version of libcdio altogether. Or we accept that 
the idea of being cross platforms means that ALL platforms have to make 
some concessions when needed, even if that adds perceived "ugliness".

Regards,

/Pete



[1] http://git.libusb.org/?p=libusb.git;a=blob;f=libusb/libusb.h
[2] 
ftp://sourceware.org/pub/pthreads-win32/prebuilt-dll-1-10-0-release/include/pthread.h