Cannot build with `--without-x` | Cannot link with libfontforge.so #465

Closed
coolwanglu opened this Issue Mar 21, 2013 · 25 comments

Comments

Projects
None yet
6 participants
Contributor

coolwanglu commented Mar 21, 2013

/usr/local/lib/../lib/libfontforge.so: undefined reference to `_FVMenuSaveAs'
/usr/local/lib/../lib/libfontforge.so: undefined reference to `GWidgetAsk8'

This bug was introduced by commit 56b687, for issue #414
autosave.c is part of libfontforge.so, which is supposed to work without GUI.

Can I add a dummy target in Makefile such that we can make sure libfontforge.so can be correctly linked with?

Contributor

JoesCat commented Mar 21, 2013

Put it back as it was before. More info in #461.

Contributor

coolwanglu commented Mar 21, 2013

@JoesCat I think #461 has nothing (directly) to do with this issue, that problematic piece of code should be moved to where in libfontforgexe but not libfontforge

Owner

mskala commented Mar 21, 2013

I'm in favour of the dummy target idea, or some equivalent test. The libfontforge library is used by more than one thing: fontforge-GUI and fontforge-non-GUI at the very least (they are effectively separate programs), and if we're calling it a library then we are claiming that it can also be used by other things as well. It's important that linking with it should work.

Contributor

coolwanglu commented Apr 18, 2013

I wonder if commit 56b687 (for issue #414) can be temporarily disabled, until libfontforge and libfontforgeexe can be clearly separated.
That piece of commit breaks the libfontforge.so since then, I wonder if #414 is as important.

Contributor

coolwanglu commented Apr 18, 2013

A new commit 6dc8c27 introduced MacServiceReadFDs to the core libarary, making it worse for this issue.

In the current situation, especially when the UI/UX issues are being focused, it's very likely that we do somethings like "Show a dialog when something happens", which actually should not be done in the core library.

Before #469 can be handled, at least I propose to add a dummy cpp file, which links to libfontforge.so, such that developer may be warned if they break the library. This dummy target is harmless and requires zero maintenance effort.

@davelab6 what do you think?

Contributor

JoesCat commented Apr 18, 2013

@coolwanglu, instead of introducing more code, what do you think about making use of the "tests" that can be performed doing build. Maybe one of the tests can be made so that if something like these #414, or 6dc8c27
or something else that can cause the library to break, get's tested, and a big warning, or halt stops the process from going further? I don't know how to do that, but testing along the way may be a way to go.

Owner

mskala commented Apr 18, 2013

I think it should have a sound effect.

More seriously: a dummy .c file linking to libfontforge.so is basically the same thing you're proposing. This wouldn't be "more code" introduced to the actual application, but rather a build system hack to make it more obvious that libfontforge is broken, when it is broken. The condition being tested is: can a binary link to libfontforge and not libfontforgeexe? If the answer is "no," then there's a problem.

Contributor

coolwanglu commented Apr 19, 2013

@JoesCat Please see #553, I intended to make something to be compiled when running make check, but didn't know how to export compilers, flags, paths etc into a scripts which can be added to testsuites. And it ended up with an normal object in the tests folder

I'm not sure if make check is a hard requirement for developers, since I see one of them failed on my machine.

Contributor

ahyangyi commented Apr 25, 2013

Could we remove the function call to MacServiceReadFDs() NOW? What is it good for, after all? Is there anything worse than a broken compilation?

Owner

davelab6 commented Apr 25, 2013

Hi everyone, sorry for my delay with looking into this. I agree with @coolwanglu and @mskala that a dummy.c would be good to prevent this regression occuring in the future - @coolwanglu, would you like to add this? :)

@monkeyiq please resolve the _FVMenuSaveAs, GWidgetAsk8 and MacServiceReadFDs :)

Contributor

coolwanglu commented Apr 25, 2013

@davelab6 It's already there, #553 :)

Owner

davelab6 commented Apr 25, 2013

Awesome! Lets merge #553 when this is resolved :)

coolwanglu referenced this issue in coolwanglu/pdf2htmlEX Apr 29, 2013

Closed

Text placement issue #126

Contributor

monkeyiq commented May 18, 2013

Fixed in #575

Contributor

monkeyiq commented May 18, 2013

The python stuff is a bit harder to separate ui/non ui. Perhaps I could have ifdef in the PyMethodDef PyFF_Font_methods declaration in python.c and the actual functions defined in pythonui.c if it is a GUI build.

monkeyiq closed this May 18, 2013

Contributor

coolwanglu commented May 18, 2013

@monkeyiq Thanks for your fix, but I'm not sure if it fixes libfontforge.so with X is enabled, still the function is involved when there is GUI. I don't have an dev environment though, I'll test it later.

Contributor

monkeyiq commented May 19, 2013

@coolwanglu it should be ok when there is a gui. The problem is that if there is no gui, then the function it wants to call doesn't even get compiled. If there is a gui, then when things get linked together the function should be available. It's not ideal code layout having the gui call there, but it should compile and work (or it did when I originally tested it with Linux/X ;)

Contributor

coolwanglu commented May 19, 2013

@monkeyiq, the problem is about linking libfontforge.so to another
program.

On Sun, May 19, 2013 at 4:30 PM, monkeyiq notifications@github.com wrote:

@coolwanglu https://github.com/coolwanglu it should be ok when there is
a gui. The problem is that if there is no gui, then the function it wants
to call doesn't even get compiled. If there is a gui, then when things get
linked together the function should be available. It's not ideal code
layout having the gui call there, but it should compile and work (or it did
when I originally tested it with Linux/X ;)


Reply to this email directly or view it on GitHubhttps://github.com/fontforge/fontforge/issues/465#issuecomment-18114243
.

Contributor

monkeyiq commented May 19, 2013

@coolwanglu yep, I'm seeing that now in a little extra testing. Also, doing import fontforge from python causes issues too :|

monkeyiq reopened this May 19, 2013

Contributor

monkeyiq commented May 19, 2013

I have a fix planned which I'll commit soon.

Contributor

coolwanglu commented May 19, 2013

@monkeyiq, I guess we need some 'plugin system' with function pointers like
in fv_interface

On Sun, May 19, 2013 at 4:52 PM, monkeyiq notifications@github.com wrote:

@coolwanglu https://github.com/coolwanglu yep, I'm seeing that now in a
little extra testing. Also, doing import fontforge from python causes
issues too :|


Reply to this email directly or view it on GitHubhttps://github.com/fontforge/fontforge/issues/465#issuecomment-18114500
.

Contributor

coolwanglu commented May 19, 2013

Thanks!

On Sun, May 19, 2013 at 4:59 PM, monkeyiq notifications@github.com wrote:

I have a fix planned which I'll commit soon.


Reply to this email directly or view it on GitHubhttps://github.com/fontforge/fontforge/issues/465#issuecomment-18114573
.

Contributor

monkeyiq commented May 19, 2013

#576

There is still some love I need to add to python.c to be cleaner too.

Contributor

monkeyiq commented May 21, 2013

Some of the UI dependant code from python.c is now split into pythonui.c and the later can augment the method table of python objects initially setup in the former.

#579

Contributor

monkeyiq commented May 21, 2013

This seems to have wrapped up the breakage I caused in the last round of collab code :( Let me know if there are still any issues I missed :)

monkeyiq closed this May 21, 2013

Contributor

coolwanglu commented May 23, 2013

Just tested. The code works perfectly now.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment