Opened 13 years ago
Closed 13 years ago
Last modified 13 years ago
#9586 closed Bug Report (Fixed)
Commit d149a01 (Changed MPUBLIC define) breaks Windows build
Reported by: | Owned by: | beirdo | |
---|---|---|---|
Priority: | trivial | Milestone: | 0.25 |
Component: | MythTV - General | Version: | Master Head |
Severity: | medium | Keywords: | MPUBLIC Windows |
Cc: | Ticket locked: | no |
Description
commit d149a01811f1976a863937d836570c59fcd04e24 (change the definition of MPUBLIC to support VS) _totally_ breaks the Windows build using gcc (cross compiled and on Windows with MSys).
PLEASE revert this change ASAP as it's simply not possible to design a simple patch to workaround this. It would be prudent to test any revised changes with the Windows packager or the mythbuild script before committing.
Attachments (10)
Change History (47)
comment:1 Changed 13 years ago by
Priority: | critical → trivial |
---|
comment:2 Changed 13 years ago by
I use my mythbuild.sh script to build for Windows which will always rebuild from scratch after a git pull, simply because the patches need to be re-applied.
Build log attached. Note the important line:
../libmythbase/util.cpp:1340: error: function ‘QString& ShellEscape(QString&)’ definition is marked dllimport
which is because libmyth is including sources from libmythbase. Note also the copious warning messages about mismatched dllimport.
Changed 13 years ago by
Attachment: | mythbuild.log.tar.bz2 added |
---|
comment:3 Changed 13 years ago by
OK, that actually pointed to a missing change from almost a month ago. I committed a fix for that particular error in 87f6a88307299 on master just now. Please try again.
comment:4 follow-up: 5 Changed 13 years ago by
A step closer, but no cigar. Build log attached. Looks like a problem with libmythtv importing MythRenderD3D9 and D3D9Image from libmythui. Ironic that it's the Windows specific code that's now broken by this Windows specific change.
Changed 13 years ago by
Attachment: | mythbuild.log.tar.2.bz2 added |
---|
comment:5 Changed 13 years ago by
Replying to Lawrence Rust <lvr@…>:
A step closer, but no cigar. Build log attached. Looks like a problem with libmythtv importing MythRenderD3D9 and D3D9Image from libmythui. Ironic that it's the Windows specific code that's now broken by this Windows specific change.
Presumably mythrender_d3d9.h just needs a sprinkling of MUI_PUBLIC in the right places.
comment:6 Changed 13 years ago by
Agreed. I sprinkled in what seemed to be missing in both mythrender_d3d9.h and mythpainter_d3d9.h. This is in 4b220b8cc7b9 on master.
comment:7 Changed 13 years ago by
v0.25pre-1204-ga5a2a9c gets further but now fails while linking libmythmetadata. Looks like mythiowrapper in libmythtv needs the same sprinkling of pixie dust ;-) New build log attached
Changed 13 years ago by
Attachment: | mythbuild.log.tar.3.bz2 added |
---|
comment:8 Changed 13 years ago by
Owner: | set to beirdo |
---|---|
Status: | new → assigned |
comment:10 Changed 13 years ago by
A bit further. Now it bombs on building mythavtest, the 1st program, New log attached.
Looks like including version.pro is the problem. With the current implementation, the apps need to link with libmythbase to access myth_source_version etc. Not ideal, as each program should have that info internally.
Don't you get the feeling that this is a fire fighting exercise? In which case we should look at the implementation. For instance, it is not necessary, just desirable, for a Windows program to mark an external function imported from a DLL as dllimport. If it's marked it saves a few bytes and is slightly faster but can complicate builds. A dll export does have to be marked though unless every extern function is exported.
Changed 13 years ago by
Attachment: | mythbuild.log.tar.4.bz2 added |
---|
comment:11 Changed 13 years ago by
I think we're almost there. I committed a fix for that in f3b2b05994aaa0. It seems to work just fine for me in Linux.
comment:12 Changed 13 years ago by
Another step closer. The attacched log shows 2 link failures to _myth_source_version
Changed 13 years ago by
Attachment: | mythbuild.log.bz2 added |
---|
comment:15 Changed 13 years ago by
OK, that's fixed but now there's a missing pthread in mythavtest main.cpp. Looks like the omission of pthread has been lurking here for some time. New log attached.
Changed 13 years ago by
Attachment: | mythbuild.log.2.bz2 added |
---|
comment:17 Changed 13 years ago by
Another day. another log ;-). This time in mythtv-setup/backendsettings.cpp:111: undefined reference to `_chanlists'. Log attached.
Changed 13 years ago by
Attachment: | mythbuild.log.3.bz2 added |
---|
comment:19 Changed 13 years ago by
A bit closer but now it complains about mythtv-setup/backendsettings.cpp:111: undefined reference to `impchanlists' Build log attached.
Changed 13 years ago by
Attachment: | mythbuild.log.4.bz2 added |
---|
comment:20 Changed 13 years ago by
hmm. Try now. I think I may have had the extern vs import/export potentially backwards.
comment:23 Changed 13 years ago by
OK, good progress. I built mythtv OK but failed in mythplugins, MythVideo?. Log attached.
Changed 13 years ago by
Attachment: | mythbuild.log.5.bz2 added |
---|
comment:25 Changed 13 years ago by
That's fixed, but another problem: mythvideo/fileassoc.cpp:41: undefined reference to `FileAssociations::file_association::file_association(). New log attached
Changed 13 years ago by
Attachment: | mythbuild.log.6.bz2 added |
---|
comment:27 Changed 13 years ago by
OK, success! It builds, runs and LiveTV, video, music and gallery all appear to work given a very brief check.
Thanks for your perseverance.
I still can't help feeling that the original change should have been tested with a Windows build or x-compile before committing.
comment:28 Changed 13 years ago by
Resolution: | → Fixed |
---|---|
Status: | assigned → closed |
comment:29 Changed 13 years ago by
Milestone: | unknown → 0.25 |
---|
comment:30 Changed 13 years ago by
Add MUI_PUBLIC to some D3D9 classes
It seems that perhaps a few classes here need to be declared to be public after moving them from libmythtv into libmythui.
Refs #9586
Changeset: 4b220b8cc7b9d7d1e9cab87c84317636d88fb73c
comment:31 Changed 13 years ago by
Add MTV_PUBLIC to a bunch of mythiowrapper protos
It seems that via libmythbluray, we are trying to import some mythiowrapper functions in libmythmeta. I added the public definitions to allow this to behave as expected with symbol visibility and mingw compiling.
Refs #9586
Changeset: eb573d29127a8b2097c02f29edf31e2a65e343f7
comment:32 Changed 13 years ago by
Another run at getting rid of symbol issues
Change version.cpp -> libs/libmythbase/version.h, which we then include in mythversion.h. Also convert from extern const char * -> #define constants. This will remove all of the symbols. However, it comes at the cost of a slightly slower re-compile as every file that uses the constants will need recompiling. If we wanted to get around that, we can use functions in libmythbase as the access points, such that only one file needs a recompile, and it is buried in a library, but still anything using said library will need relinking.
Can't win, really. Refs #9586
Changeset: 70a157f96c5644510700057cb9437395188c67d9
comment:33 Changed 13 years ago by
Fix yet another symbol visibility miss
This time in the frequency table not being exported for mythtv-setup to see. Refs #9586
Changeset: 52c0835c9255233ba5ffde5aff447e7168ac6e7c
comment:34 Changed 13 years ago by
Take 2 on the freqtable symbol vis.
I think I had the magic backwards, perhaps. If this doesn't fix it, I'm not sure how to proceed. Refs #9586
Changeset: f2ed11dcac7f91c1a326c6b6658d3e4bceb3bc3d
comment:35 Changed 13 years ago by
Rename frequencies.c -> frequencies.cpp
Since the MTV_PUBLIC only applies to C++ source, we effectively were not ever exporting the chanlists[] table at all. The code in the file compiles just fine as C++, so there should be no harm in doing this.
Refs #9586
Changeset: d24531b7f3802ffeb8482eece97d441d4e1063dd
comment:36 Changed 13 years ago by
Missing META_PUBLIC for VideoMetadata::SortKey?
Seems that subclasses don't inherit the export from the parent classes?
Refs #9586
Changeset: ffd529f1b671f4dce0f1f40350d365f0ce7644fe
comment:37 Changed 13 years ago by
Added META_PUBLIC to another subclass.
Seems another public statement was missing. As an interesting note, the "struct file_association" should likely be a "class file_association" if it has members that are method functions.
Refs #9586
Changeset: 24484fd3109260eeb583ca699a24967dd41cba4a
It would be a good idea to be sure to do the following first before considering backing anything out:
Also, with no error messages or anything to go on, I'd be hesitant to back anything out.
Also: please do not set the priority field.