Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix build on macos #26

Merged
merged 3 commits into from
Aug 30, 2022
Merged

Fix build on macos #26

merged 3 commits into from
Aug 30, 2022

Conversation

AkiSakurai
Copy link
Contributor

  1. times on macos do not accept NULL
  2. There is system macro "howmany" in macos header, rename the function to avoid conflicts
  3. Add missing EXTRA_CPPFLAGS to some makefiles
  4. There is no strchrnul on macos
  5. Fix configure script to forward arguments correctly
  6. Do not link laxkit in plugin

1. times on macos do not accept NULL
2. There is system macro "howmany" in macos header, rename the fucntion to avoid conflicts
3. Add missing EXTRA_CPPFLAGS to some makefiles
4. There is no strchrnul on macos
5. Fix configure script to forward arguments correctly
6. Do not link laxkit in plugin
@tomlechner
Copy link
Contributor

Wow, you are going deep, my build system needs the attention!
Building for a mac has been requested many times, so this is looking very good.

I merged your companion Laxkit pr Laidout/laxkit#8.
Some minor notes for this pr:

  1. Line 570 in src/interfaces/pagerangeinterface.cc should probably be the following, since part might be nullptr. Never crashed on me before since the old faulty part>0, while not doing what intended, didn't segfault!
    if (pos >= 0 || r >= 0 || (part && *part > 0)) return 1;

  2. plugins/addonactions.o should be grouped with plugins/plugin.o in src/Makefile.

  3. The AddonAction class is not really used yet, I think I accidentally last pushed without defining all the functions. It is not supposed to be an abstract class. I have some unpushed changes not quite ready for pushing that defines it a little more. It's ok to leave as is, it doesn't break anything, and I can adjust after merging your pr.

  4. I need to fix the travis pull error, I probably need to update tokens. I've been meaning to revamp CI related things before the next Laidout version, so I've opened a new issue Revamp ci #27 if you are interested.

If you can adjust the pr for 1 and 2, then I can merge.

…/Makefile

add missing null check
fix install language file
@tomlechner
Copy link
Contributor

Looks good, thank you!

@tomlechner tomlechner merged commit 9c53263 into Laidout:master Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants