Open / Save to cloud on Windows 10

[Jérémie Guichard]

Hey guys,

I narrowed it!

It took me a little while to get the windows dev env in place, but
instructions in packaging/windows/mxe-based-build.sh were very helpful.
Only one little call to curl's buildconf script is missing in
mxe-based-build.sh but once done manually the rest went smoothly. I may fix
this in a separate commit, maybe together with automatically cloning
dependencies from github...
It "only" took 8h to build mxe inside the vm, thanks to the warning in the
comments and from Dirk's last message, I started the build before going to
bed :)

Back to the non-ASCII user name/path for local checkout of the repository,
the issue is caused by the call to stat(sys/stat.h) that do not support
non-ASCII paths. I could not find a place that was stating it explicitly
but seems reasonable explanation. Since the cloning and checkout itself
works in libgit2 I made some digging in that code and there a set of
portability functions are used (p_stat) with simple mapping to system's
stat() for posix systems and specific implementations using win32 APIs for
win case.

As a proof of concept I added an ugly: "extern int p_stat(const char* path,
struct stat* buf);" on top of git-access.c and used it in get_remote_repo
and is_git_repository (these are the 2 only places stat function is used in
the whole project). It fixed my issue (I could Open and Save could
storage)...

Now what should we take as a fix:
1. The ugly extern
Cons
I call it ugly since (in libgit2) that function is not actually exposed in
the headers, not sure how Android and iOS apps are built (actually
linked...) so it could caused some issues there.
I have to mention the "bad design" / hidden dependency to libgit2 internals
argument too.

Pros
On the other hand it is quite an easy fix for the time we do not encounter
other compatibility issues in other places of the soft.
It is (currently) limited to *git*-access.c so (if it does not raise issues
on other platforms/targets) having a tight coupling to libgit2 internals
could be acceptable.

2. Implement our own portable is_dir
In all the places stat function is used, it is to check if the path is a
directory (and exists). Going through a portable stat is a bit "overkill"
(processing to provide all the elements that output "struct stat" needs)
when we only need to know if we have a directory. Not that this part of the
code is actually perf critical, it is always good to ask ourselves the
question.
Plus it makes the code a bit more readable. I think this option is my
favorite and would avoid issues in other platforms (if they exist at all).

3. Implement our own portable stat
The choice between this option and the previous is_dir proposal is always
hard when it comes to portability (specifically with windows).
Should you port system calls or should you write client code using higher
level abstractions is always a big debate.
Furthermore, in bigger projects that use multiple 3rd party libs the
portage is often already done in several of the included libs, so writing
yet another port results in adding code that typically already exists
multiple times in your final binary...
I'm more often working with C++ so I tend to love abstractions, when it
comes to C the choice is a bit harder.
Subsurface core is, from my point of view, higher level code so I would go
for abstraction, but I'm curious to hear about other point of views.

3. Other proposal?
There could be other solutions I have overseen, do you guys think about
something else?


I'm still a bit fresh with this project and do not have yet an overview of
all use cases, platforms and targets that are involved, this is why I
preferred to ask before proposing a pull request with changes that could
have side effects...
On a related point, I have not looked yet on how testing is done, (and how
far it goes) I would be more than happy to extend the test with this use
case, I'd only need a bit of guidance.

While we are talking about porting, is there a thread I could look into to
understand the choice that was made to go for cross compilation rather than
building on target? cmake is quite helpful in that regard. Maybe the main
reason is that most developer do not want to install a win VM to check
their code do build there :)

Cheers,

Jeremie



2017-02-19 23:24 GMT+07:00 Dirk Hohndel <dirk at hohndel.org>:

> On Sun, Feb 19, 2017 at 05:32:15PM +0700, Jérémie Guichard wrote:
> >
> > Computer is Suunto Zoop. With Suunto DM5 I have to sync multiple times
> > before all logs are synced...
>
> Yeah, DM5 is a very special piece of s... err... software.
>
> > I'll take a look into translations, I normally use English interfaces,
> just
> > switched to french to see the status, seems pretty complete though some
> > translations could be improved. I guess I'm a bit late for the 4.6.2...
> > Website seems to be a bit more behind...
>
> Yes, 4.6.2 will hopefully go out today.
>
> It's funny, I can't stand translated UIs, either, but our data tell us
> that more than half our users indeed use translated UIs...
>
> > Indeed I used official 4.6.1 installer.
> >
> > No specific ACLs on the directory. But my user name do contains the
> french
> > "é". So I created a new user with only ASCII chars, cloud storage
> features
> > Open/Save are working fine. You have found the issue, yepee!!!
>
> Excellent. So we have a workaround.
>
> > How should I proceed? ticket, fix, etc. I must admit I'm not there yet
> with
> > setting up dev env, I only got to build from source yesterday in the
> ubuntu
> > virtual machine... (btw I had a short attempt to do so using linux
> > subsystem for windows but did not succeed...)
>
> Building under Windows is incredibly painful. Lubomir was the only person
> who every got it to work reliably, and I think even he ran into problems
> lately. There is a file packaging/windows/mxe-based-build.sh that should
> allow you to cross build a Windows installer under Linux. Read the file
> for some of the setup you need to do to make this work. Even on a fast
> system, the first run through takes quite a while - but it gets better
> once all the libraries are built.
>
> I would love to see another developer with Windows background (and an
> interest in Windows as their main platform) start contributing. As it
> happens, almost all of the developers run Linux or Macs as their platform
> of choice, and Lubomir, our main Windows developer (without whom there
> really wouldn't be a functional Windows version) is actually not a diver
> and only does this for the love of programming...
>
> /D
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20170220/55f68d98/attachment-0001.html>