Skip to content
This repository has been archived by the owner on Mar 23, 2020. It is now read-only.

Reduce submodule usage #2

Open
jet082 opened this issue Oct 13, 2019 · 7 comments
Open

Reduce submodule usage #2

jet082 opened this issue Oct 13, 2019 · 7 comments

Comments

@jet082
Copy link

jet082 commented Oct 13, 2019

I have a fully working version of TIC-80 for iOS. However, since this required editing the source code of a few submodules, it is not something I can just simply submit a pull request for.

The modules in question are sdl2 and squirrel within the 3rd-party submodule package. Additionally, the lua version in the 3rd-party submodule is out of date and needs one edit.

To fix this in the current way I would need pull requests upstream for sdl2 (which seems very unlikely, given how large that project is), and squirrel (which seems even more unlikely as that has not updated in years). Not to mention the 3rd-party submodule itself to fix the lua issue.

As a result, it seems unlikely, with the current structure, that this repository will literally ever work on iOS at literally any point in the future.

Here is my working repository. I am not saying we need to make EVERYTHING that is a submodule into an integrated part of the repository, but we need to consider making SOME - particularly of the ones (like squirrel) that literally have not updated in years.

https://github.com/Jet082/TIC-80/

@RobLoach
Copy link
Collaborator

The modules in question are sdl2 and squirrel within the 3rd-party submodule package.

Could possibly disable Squirrel for this use case until Squirrel gets updated. Since the submodules are handled over at https://github.com/nesbox/3rd-party , we could update the submodule to reference our own fork until the change is merged upstream. Here is how that workflow would look....

  1. Visit the repo you want to fork from https://github.com/nesbox/3rd-party . Let's take SDL-mirror/SDL as an example.
  2. Fork https://github.com/SDL-mirror/SDL to your own org (jet082/SDL)
  3. Make the needed changes in jet082/SDL
  4. Create a pull request (either to upstream, or to your own fork, to help track changes)
  5. Update the submodule in https://github.com/nesbox/3rd-party to reference your fork (I've found deleting the submodule, commit, and recreating a new one helpful to change the remotes)
  6. Create a pull request to 3rd-party with the submodule change

squirrel (which seems even more unlikely as that has not updated in years)

Looks like there have been updates in Squirrel in the ppast couple months, I'll make an issue to update it in TIC-80. https://github.com/albertodemichelis/squirrel/commits/master

would need pull requests upstream for sdl2

What extent are the changes needed in SDL? Are there compilation flags we could toggle for it?

I am not saying we need to make EVERYTHING that is a submodule

I've found the submodules make it really easy to update the dependencies. I've updated Duktape, and made changes to how SDL is configured, with success. While the workflow I've listed above does take a bit more time than copy/pasta, and duplicating the code base, it does save maintenance hell down the line.

@jet082
Copy link
Author

jet082 commented Nov 16, 2019

So, here are the specific changes needed to get tic80 to work.

MAIN BRANCH CHANGES

  1. Add the following file to the root of the directory: https://github.com/jet082/TIC-80/blob/master/ios.cmake

  2. Make the following changes to CMakeLists.txt

option(BUILD_SDL "SDL Enabled" ON)

to

if (IOS)
        set(BUILD_DEMO_CARTS_DEFAULT OFF)
        set(BUILD_SOKOL_DEFAULT OFF)
        set(BUILD_PLAYER_DEFAULT OFF)
        option(BUILD_SDL "SDL Enabled" OFF)
        add_definitions(-DIOS)
else()
        option(BUILD_SDL "SDL Enabled" ON)
endif()

______________________

Add

        if(IOS)
                set_target_properties(tic80_libretro PROPERTIES SUFFIX "_ios.dylib")
        endif()

beneath the similar android declaration
  1. include/tic80_config.h
Uncomment the lines:

// #	ifndef TARGET_OS_IPHONE
...
// #	endif /* TARGET_OS_IPHONE */

SUBMODULE CHANGES

Submodule: Lua

  1. 3rd-party/lua/loslib.c:
Add the following to the top:

#ifdef IOS
	#include <spawn.h>
	extern char **environ;
#endif

Change

static int os_execute (lua_State *L) {
  const char *cmd = luaL_optstring(L, 1, NULL);
  int stat = system(cmd);
  if (cmd != NULL)
    return luaL_execresult(L, stat);
  else {
    lua_pushboolean(L, stat);  /* true if there is a shell */
    return 1;
  }
}

to

static int os_execute (lua_State *L) {
    const char *cmd = luaL_optstring(L, 1, NULL);
    pid_t pid;
    char* argv[]  =
    {
        cmd,
        NULL
    };
    int result = posix_spawn(&pid, argv[0], NULL, NULL, argv, environ);
    waitpid(pid, NULL, 0);
    if (cmd != NULL)
        return luaL_execresult(L, result);
    else {
        lua_pushboolean(L, result);  /* true if there is a shell */
        return 1;
    }
}

SUBMODULE: SQUIRREL

  1. 3rd-party/squirrel/sqstdlib/sqstdsystem.cpp
Add the following to the top

#ifdef IOS
	#include <spawn.h>
	extern char **environ;
#endif

Change:

static SQInteger _system_system(HSQUIRRELVM v)
{
    const SQChar *s;
    if(SQ_SUCCEEDED(sq_getstring(v,2,&s))){
        sq_pushinteger(v,scsystem(s));
        return 1;
    }
    return sq_throwerror(v,_SC("wrong param"));
}

to

static SQInteger _system_system(HSQUIRRELVM v)
{
    const SQChar *s;
    if(SQ_SUCCEEDED(sq_getstring(v,2,&s))){
	#ifdef IOS
		pid_t pid;
		posix_spawn(&pid, s, NULL, NULL, NULL, environ);
		sq_pushinteger(v, 0);
	#else
	        sq_pushinteger(v,scsystem(s));
	#endif
        return 1;
    }
    return sq_throwerror(v,_SC("wrong param"));
}

Then make a directory called build2 in root and run cmake .. -DCMAKE_TOOLCHAIN_FILE=../ios.cmake -DPLATFORM=OS64; make from that directory. The dylib will be in the build2/lib directory.

@jet082
Copy link
Author

jet082 commented Nov 16, 2019

Now, obviously we can do the in-house stuff easily here. And, maybe we could get squirrel to add in IOS flags.

HOWEVER, the lua case is impossible.

I happen to know that several people now have, over many years, tried to get the lua people to change this specific set of lines of code so as to allow iOS to compile. And yet it remains unchanged. So, we need to make the lua code local (or downgrade the lua to a version that did historically work). People have been complaining about this since 2017 at least: http://lua.2524044.n2.nabble.com/Lua-source-does-not-build-for-iOS-since-recent-XCode-Update-td7679687.html. Other projects with iOS support and Lua have done the same as I am suggesting: https://github.com/danomatika/ofxLua/blob/master/README.md.

@jet082
Copy link
Author

jet082 commented Nov 16, 2019

Additionally, other libretro cores - stonesoup, mame, lutro, desmume, gw, mesen, daphne, mame2016, thepowdertoy, and even libretro-deps (which chailove uses, incidentally) keep LOCAL copies of lua so that modifications like this can enable support for devices like iOS.

@jet082
Copy link
Author

jet082 commented Nov 16, 2019

I have made a PR over at squirrel. We will see if it gets accepted. However, we should not wait on lua changes - I doubt they will come anytime soon.

albertodemichelis/squirrel#204

@clsource
Copy link

Hello I created this fork to address this issue with lua, and add some additions like the += assignment.

https://github.com/NinjasCL/lua8

:)

@RobLoach
Copy link
Collaborator

@clsource Looks like a great addition! Mind creating an issue in the upstream repo at https://github.com/nesbox/TIC-80 ? Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants