kometbomb / klystrack

A chiptune tracker
http://kometbomb.github.io/klystrack/
Other
481 stars 29 forks source link

Issue building klystrack on Linux #295

Closed distek closed 3 years ago

distek commented 3 years ago

Hello,

I found your repo and wanted to give it a try! However, during the build I received this:

/usr/bin/ld: objs.debug/command.o:/home/distek/git-clones/klystrack/klystron/src/gui/menu.h:36: multiple definition of `menu_t'; objs.debug/action.o:/home/distek/git-clones/klystrack/klystron/src/gui/menu.h:36: first defined here

for all of the .o files during linking.

I was able to resolve this by changing the menu_t definition in menu.h to the following:

struct menu_t
{
    int flags;
    const struct menu_t *parent;
    const char * text;
    const struct menu_t *submenu;
    void (*action)(void*, void*, void *);
    void *p1, *p2, *p3;
};

The only change being the removal of menu_t after the struct definition:

} menu_t;

I don't know if this would cause issues on other platforms, so I'm just opening this issue for now. If for no other reason than to help someone else using Linux to be able to build this successfully.

Thank you for what you do.

fgaz commented 3 years ago

fwiw, I'm on linux too and klystrack builds both with and without the patch

rofl0r commented 3 years ago

it's usually a bad idea to declare a global variable from a header that's included from multiple translation units. the right fix would be to declare only the struct type in the header with a reference to an external instance, which then lives in a single TU. but since @fgaz reports that klystrack builds with the patch, the variable menu_t is probably not even used.

kometbomb commented 3 years ago

I think it's safe to say that is just a typo and missing the typedef keyword.

to 24. syysk. 2020 klo 13.44 rofl0r (notifications@github.com) kirjoitti:

it's usually a bad idea to declare a global variable from a header that's included from multiple translation units. the right fix would be to declare only the struct type in the header with a reference to an external instance, which then lives in a single TU. but since @fgaz https://github.com/fgaz reports that klystrack builds with the patch, the variable menu_t is probably not even used.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/kometbomb/klystrack/issues/295#issuecomment-698266002, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIIV2L6MY4UDZ2C6I6PCE3SHMPG7ANCNFSM4QKYA3PQ .

rofl0r commented 3 years ago

I think it's safe to say that is just a typo and missing the typedef keyword.

in this case it would make sense that @distek files a PR with his patch.

kometbomb commented 3 years ago

Agreed.

to 24. syysk. 2020 klo 22.10 rofl0r (notifications@github.com) kirjoitti:

I think it's safe to say that is just a typo and missing the typedef keyword.

in this case it would make sense that @distek https://github.com/distek files a PR with his patch.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kometbomb/klystrack/issues/295#issuecomment-698534353, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIIV2JVHSZU3LDMZGVGHADSHOKTJANCNFSM4QKYA3PQ .

kometbomb commented 3 years ago

Merged. Thanks!