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

Windows support #5

Closed
becvert opened this issue Jun 8, 2016 · 29 comments
Closed

Windows support #5

becvert opened this issue Jun 8, 2016 · 29 comments

Comments

@becvert
Copy link

becvert commented Jun 8, 2016

Hi @akofman
I think one cannot prepare their project on Windows as find . -name ... isn't a valid command.
It'd be nice though if we could.
Thank you

akofman added a commit that referenced this issue Jun 8, 2016
@akofman
Copy link
Owner

akofman commented Jun 8, 2016

Indeed, it should be better with this last commit. Please let me know if you could test it, I don't have any Windows installed :(

@becvert
Copy link
Author

becvert commented Jun 9, 2016

glob dependency is missing in package.json

glob('*Bridging-Header*.h', { cwd: pluginsPath }, function(error, files) {...} is not exploring the tree

It should be either:

glob('**/*Bridging-Header*.h', { cwd: pluginsPath }, function(error, files) {...}

or

glob('*Bridging-Header*.h', { cwd: pluginsPath, matchBase: true }, function(error, files) {...}

otherwise it's all OK! Thank you!

@akofman
Copy link
Owner

akofman commented Jun 9, 2016

glob is bundled with cordova-lib , so I should not need to add it as a dependency, Did you notice any error because of that ?

@becvert
Copy link
Author

becvert commented Jun 9, 2016

Indeed. I've had a module not found though. Let me double check that.

Also there is an issue with the Bridging Header Path property being set in the Windows format.
While I prepare my project on Windows, I do build the ios platform with xcode obviously so this path should be kept in the osx format.

@akofman
Copy link
Owner

akofman commented Jun 9, 2016

To understand well, how can you use xcode from Windows ? Are you using a VM and keeping your sources on your local file system ?
Sorry , I don't really get your last point actually :/

@becvert
Copy link
Author

becvert commented Jun 9, 2016

I meant to say that I prepare all platforms from Windows,
and I compile ios on a Mac via a mounted network share :)
Anyway the SWIFT_OBJC_BRIDGING_HEADER property should be $(PROJECT_DIR)/$(PROJECT_NAME)/Bridging-Header.h( which is what cordova is now setting by default I believe) instead of an absolute path.

About glob dependency I do have a module not found. Don't recall how the whole chain of dependency is working in node.
You might have had the same issue with the xcode module because it is bundled with cordova-lib as well but you added it in your package.json

@akofman
Copy link
Owner

akofman commented Jun 9, 2016

Hmmm interesting, let me check what happens without an absolute path.

yep, xcode is also bundled with cordova-lib but with an old version which has been recently updated. So in order to keep the compatibility with all Cordova versions, I bundled it.
What version of Cordova are you using ?

@becvert
Copy link
Author

becvert commented Jun 9, 2016

cordova 6.1.1

without an absolute path, fs.stat will always fail I think. that's the problem

@becvert
Copy link
Author

becvert commented Jun 9, 2016

is it working on your side with just a simple require('glob')?
I'm pretty sure you shoud either require('cordova/node_modules/cordova-lib/node_modules/glob') or add it explicitly as a dependency of the plugin.

@akofman
Copy link
Owner

akofman commented Jun 9, 2016

Ah ! Yep I should use context.requireCordovaModule('glob'). Let me commit that.

akofman added a commit that referenced this issue Jun 9, 2016
@akofman
Copy link
Owner

akofman commented Jun 9, 2016

Should be better right ?

@becvert
Copy link
Author

becvert commented Jun 9, 2016

about the dependency, yes very good!

@becvert
Copy link
Author

becvert commented Jun 9, 2016

Cordova does not seem to set SWIFT_OBJC_BRIDGING_HEADER at all. I apologize.

@akofman
Copy link
Owner

akofman commented Jun 9, 2016

Yep that's why I do the job ;)

@becvert
Copy link
Author

becvert commented Jun 9, 2016

:)

what about:

1 - when you look for the header file you use the SWIFT_OBJC_BRIDGING_HEADER property first and if fs.stat fails then you look for a 'Briding-Header.h' file in the projectPath. Cordova created a default header file there anyway.

2 - when you do create the Bridging-Header.file you set SWIFT_OBJC_BRIDGING_HEADER to $(PROJECT_NAME)/Bridging-Header.h. No absolute path.

@akofman
Copy link
Owner

akofman commented Jun 9, 2016

Yep, that's exactly what I've done. Please let me know if all is fine with this last commit.

@becvert
Copy link
Author

becvert commented Jun 9, 2016

That's almost it.

Path separator on Windows is \.
So bridgingHeaderPath.split(platformPath + '/')[1] does not work on Windows , bridgingHeaderPath.split(platformPath + '\\')[1] does.
But anyway xcode does not handle properly myproject\Bridging-Header.h which it interprets as myprojectBridging-Header.h (removed separator)
That's why, If I may, I would literally set $(PROJECT_NAME)/Bridging-Header.h instead.

Also Im' not sure about that:

if(bridgingHeaderPath !== undefined) {
      bridgingHeaderPath = path.join(platformPath, bridgingHeaderPath);
}

It prevents people to use an absolute path if they wish so. I would get rid of this.

Please allow me again, I think we need something like that:

bridgingHeaderPath = unquote(xcodeProject.getBuildProperty('SWIFT_OBJC_BRIDGING_HEADER'));
try {
    fs.statSync(bridgingHeaderPath);
} catch(err) {
    bridgingHeaderPath  = path.join(projectPath, 'Bridging-Header.h')
    try {
        fs.statSync(bridgingHeaderPath);
        xcodeProject.updateBuildProperty('SWIFT_OBJC_BRIDGING_HEADER', '$(PROJECT_NAME)/Bridging-Header.h');
    } catch (err) {
        // create header file here...
        xcodeProject.updateBuildProperty('SWIFT_OBJC_BRIDGING_HEADER', '$(PROJECT_NAME)/Bridging-Header.h');
    }
}

what do you think? thanks ;)

@becvert
Copy link
Author

becvert commented Jun 9, 2016

Just for your information, CB-10072, in platforms\ios\cordova\build.xcconfig

SWIFT_OBJC_BRIDGING_HEADER = $(PROJECT_DIR)/$(PROJECT_NAME)/Bridging-Header.h

@akofman
Copy link
Owner

akofman commented Jun 9, 2016

Yep he also created an issue on this project about that jira.

@akofman
Copy link
Owner

akofman commented Jun 9, 2016

But it doesn't ensure the old cordova versions compatibility and I'd like to keep that.

@becvert
Copy link
Author

becvert commented Jun 9, 2016

ok I understand your points.
so instead I guess you could replace all path.join with path.posix.join
so that the path separator is always a forward slash, even on windows.
windows can handle that.
and then that's it.

@akofman
Copy link
Owner

akofman commented Jun 9, 2016

What about this last commit ?

@becvert
Copy link
Author

becvert commented Jun 9, 2016

any custom SWIFT_OBJC_BRIDGING_HEADER property will (re)create the file?
it's fine with me.
still need posix in getBridgingHeaderPath

@akofman
Copy link
Owner

akofman commented Jun 9, 2016

The path to the bridging header is now forced to $(PROJECT_DIR)/$(PROJECT_NAME)/Bridging-Header.h or $(PROJECT_DIR)/$(PROJECT_NAME)/Plugins/Bridging-Header.h depending on the Cordova version.
If an already SWIFT_OBJC_BRIDGING_HEADER is configured and different from these paths, it will be overriden.

akofman added a commit that referenced this issue Jun 9, 2016
@akofman
Copy link
Owner

akofman commented Jun 9, 2016

Are we all good now ? 🙏 🙏 🙏

@becvert
Copy link
Author

becvert commented Jun 10, 2016

That's good for me 👍
Awesome job! congrats! 👏

@akofman
Copy link
Owner

akofman commented Jun 10, 2016

Oh yeah ! Let's merge it ;)

@akofman
Copy link
Owner

akofman commented Jun 10, 2016

And thanks for your precious reviews.

akofman added a commit that referenced this issue Jun 10, 2016
akofman added a commit that referenced this issue Jun 10, 2016
@becvert
Copy link
Author

becvert commented Jun 10, 2016

you're welcome 😌
I will now use your plugin in my own cordova plugins

@akofman akofman closed this as completed Jun 10, 2016
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

No branches or pull requests

2 participants