Re: Open / Save to cloud on Windows 10

2017-02-23 Thread Dirk Hohndel 
Hi Jérémie

On Thu, Feb 23, 2017 at 07:31:20PM +0700, Jérémie Guichard wrote:
> Hi guys,
> 
> Pull request sent (https://github.com/Subsurface-divelog/subsurface/pull/222
> )
> Sorry for the late reply, these days were quite busy with a bit of coding
> but mainly with my Instructor Training Course :)

It didn't seem late at all. Usually it takes people a few days to
implement things that go beyond a "quick fix".

> Based on your recommendations I've added a subsurface_stat portability
> function. There I used the corresponding widechar implementation wstat()
> for windows (after utf8 to utf16 conversion) and simply called the normal
> stat() otherwise. It seems to work nicely.

And sounds very reasonable.

> From what I understand when reading msdn, stat() is actually expecting a
> path encoded with the codepoint that is defined in system settings so it
> can be many things but rarely utf8... Win32 only apps do not face issues
> since they would read this from user input or other win32 api that are
> already returning paths/stings encoded with that codepoint. In our case
> everything is converted to utf8 before being returned to the app level. At
> least this is my current understanding of the issue we faced.
> 
> Since I like to work with tests, I made some little changes to the testing
> code in order to be able to run cross compiled tests under windows too. And
> added a case that test specificaly our issue.
> The changes I've made there are not yet providing a fully flavored solution
> (but it is a step forward, that allowed me to test without too much
> troubles).

I like the way you are thinking. We could go one step further and include
a non-standard path in our source tree and access this on all platforms.
This would still require you to set things up the right way when cross
compiling, but would make it one step closer. So add something like

dives/téßtænç/ģïffé₁₂.ssrf

and load that.
 
> What could come next (but I prefer to do this in a separate change) would
> be:
> - Packaging of the tests together with dependencies and test data for
> windows run
>  (currently I went the hard way of manually copying what I needed)

It should be easy enough to have an option to include the tests in the
existing installer - just use a different .nsi to start with, or modify
the .nsi.in to get what you need

> - Making test execution not depending on user name. (All cloud part of
> TestGitStorage implementation uses default path based on user dir to
> execute tests.)

So there are different things to consider here. TestGitStorage should
certainly test the same code that would be run by Subsurface. But you
could add a TestNonAscii which uses the file suggested above and creates
git storage below a similarly creatively named folder.

> - Make test data user independent. (When I run the tests I get many:
> "didn't find picture entry for
> /home/hohndel/subsurface/dives/images/PA102039.jpg"
> caused by hard coded path in the test cases)

Yeah, I noticed this last weekend as well, when running tests on a fresh
VM. That certainly should be fixed in the test data.

> - I still could not run TestPreferences.exe I get a strange missing
> dependency message (certainly explained by my manual copy of the
> dependencies ;))
> - Some issue with recently added TestMerge and TestParse caused by a
> trailing \r in the value from written file
>Actual   (readin.takeFirst()) : " version='3'>"
>Expected (written.takeFirst()): " version='3'>\r"

We should ignore line endings when comparing the files.

> For now I got TestGitStorage to:
> 1. Reproduce the issue when the fix was not there. see log below, I cut off
> the missing image messages
> 2. Pass once the fix is included. see log below too
> 
> Have a look at the pull request, I will be happy to make any improvements
> or fixes to the change. (I will not be unhappy if you pull it in as is)

I'll look at that next.

Thanks for working on this!

/D
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Re: Open / Save to cloud on Windows 10

2017-02-23 Thread Jérémie Guichard 
Hi guys,

Pull request sent (https://github.com/Subsurface-divelog/subsurface/pull/222
)
Sorry for the late reply, these days were quite busy with a bit of coding
but mainly with my Instructor Training Course :)

Based on your recommendations I've added a subsurface_stat portability
function. There I used the corresponding widechar implementation wstat()
for windows (after utf8 to utf16 conversion) and simply called the normal
stat() otherwise. It seems to work nicely.

>From what I understand when reading msdn, stat() is actually expecting a
path encoded with the codepoint that is defined in system settings so it
can be many things but rarely utf8... Win32 only apps do not face issues
since they would read this from user input or other win32 api that are
already returning paths/stings encoded with that codepoint. In our case
everything is converted to utf8 before being returned to the app level. At
least this is my current understanding of the issue we faced.

Since I like to work with tests, I made some little changes to the testing
code in order to be able to run cross compiled tests under windows too. And
added a case that test specificaly our issue.
The changes I've made there are not yet providing a fully flavored solution
(but it is a step forward, that allowed me to test without too much
troubles).

What could come next (but I prefer to do this in a separate change) would
be:
- Packaging of the tests together with dependencies and test data for
windows run
 (currently I went the hard way of manually copying what I needed)
- Making test execution not depending on user name. (All cloud part of
TestGitStorage implementation uses default path based on user dir to
execute tests.)
- Make test data user independent. (When I run the tests I get many:
"didn't find picture entry for
/home/hohndel/subsurface/dives/images/PA102039.jpg"
caused by hard coded path in the test cases)
- I still could not run TestPreferences.exe I get a strange missing
dependency message (certainly explained by my manual copy of the
dependencies ;))
- Some issue with recently added TestMerge and TestParse caused by a
trailing \r in the value from written file
   Actual   (readin.takeFirst()) : ""
   Expected (written.takeFirst()): "\r"

For now I got TestGitStorage to:
1. Reproduce the issue when the fix was not there. see log below, I cut off
the missing image messages
2. Pass once the fix is included. see log below too

Have a look at the pull request, I will be happy to make any improvements
or fixes to the change. (I will not be unhappy if you pull it in as is)

Cheers,

Jeremie

Before:
* Start testing of TestGitStorage *
Config: Using QtTest library 5.7.0, Qt 5.7.0 (i386-little_endian-ilp32
shared (dynamic) release build; by GCC 4.9.4)
PASS   : TestGitStorage::initTestCase()
PASS   : TestGitStorage::testSetup()
PASS   : TestGitStorage::testGitStorageLocal(ASCII path)
FAIL!  : TestGitStorage::testGitStorageLocal(Non ASCII path) Compared
values are not the same
   Actual   (save_dives(qUtf8Printable(testDirName + "[test]"))): -1
   Expected (0) : 0
/home/jeremie/dev/subsurface-win/subsurface/tests/testgitstorage.cpp(81) :
failure location
FAIL!  : TestGitStorage::testGitStorageCloud() Compared values are not the
same
   Actual   (parse_file(qUtf8Printable(cloudTestRepo))): -1
   Expected (0): 0
/home/jeremie/dev/subsurface-win/subsurface/tests/testgitstorage.cpp(107) :
failure location
FAIL!  : TestGitStorage::testGitStorageCloudOfflineSync() Compared values
are not the same
   Actual   (parse_file(qUtf8Printable(localCacheRepo))): -1
   Expected (0) : 0
/home/jeremie/dev/subsurface-win/subsurface/tests/testgitstorage.cpp(129) :
failure location
FAIL!  : TestGitStorage::testGitStorageCloudMerge() Compared values are not
the same
   Actual   (parse_file(qUtf8Printable(localCacheRepoSave))): -1
   Expected (0) : 0
/home/jeremie/dev/subsurface-win/subsurface/tests/testgitstorage.cpp(179) :
failure location
FAIL!  : TestGitStorage::testGitStorageCloudMerge2() Compared values are
not the same
   Actual   (parse_file(qUtf8Printable(localCacheRepo))): -1
   Expected (0) : 0
/home/jeremie/dev/subsurface-win/subsurface/tests/testgitstorage.cpp(227) :
failure location
FAIL!  : TestGitStorage::testGitStorageCloudMerge3() Compared values are
not the same
   Actual   (parse_file(qUtf8Printable(cloudTestRepo))): -1
   Expected (0): 0
/home/jeremie/dev/subsurface-win/subsurface/tests/testgitstorage.cpp(284) :
failure location
PASS   : TestGitStorage::cleanupTestCase()
Totals: 4 passed, 6 failed, 0 skipped, 0 blacklisted, 20258ms
* Finished testing of TestGitStorage *

After:
* Start testing of TestGitStorage *
Config: Using QtTest library 5.7.0, Qt

Re: Open / Save to cloud on Windows 10

2017-02-20 Thread Linus Torvalds 
On Mon, Feb 20, 2017 at 9:41 AM, Dirk Hohndel  wrote:
>
> How about writing a wrapper function that can be called from C in
> qthelper.cpp (we have quite a few of them there already) and have that do
> something like
>
> extern "C" bool dir_exists(const char *dir)
> {
> return QDir(dir).exists();
> }

So we do actually want more than to check if it's a directory. We use
"stat()" for two things in the git wrapper:

 - to check the existing cache:

if it's a directorty, we uupdate it

 - if it's a non-directory,. we do warnings/cleanup

 - if it doesn't exist, we may create it.

So that code actually wants at least a ternary return value, or just a
regular stat() like it does now.

Then in is_git_repository() we do indeed just want to know "does this
directory exist".

We *could* just use our existing "subsurface_open() + fstat()", which
is already a pattern we use, but doing "open()" on a directory ends up
being one of those things I could easily see not working under Windows
either.

 Linus
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Re: Open / Save to cloud on Windows 10

2017-02-20 Thread Dirk Hohndel 
On Mon, Feb 20, 2017 at 09:42:35AM -0800, Linus Torvalds wrote:
> On Mon, Feb 20, 2017 at 6:49 AM, Jérémie Guichard  wrote:
> >
> > I narrowed it!
> 
> Good job.
> 
> > 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)...
> 
> Ugh. Ok. So it appears that whatever Windows posix emulation library
> we use is pretty broken.  Looking at the "p_stat()" implementation in
> libgit2, it just does a utf8->wchar expansion.
> 
> We actually do the same thing in "subsurface_open()", for all the same
> reasons. I wonder why the system "stat()" routine can't just get that
> right? What the hell is wrong with Windows libraries? Why doesn't the
> windows "stat()" routine just DTRT?
> 
> > 3. Implement our own portable stat
> 
> I think this is the right thing to do, we already end up having all
> the pieces, and we already use "fstat()" even on windows, so we could
> just do a subsurface_stat() exactly like we do subsurface_open().

Ah, two conflicting pieces of advise from the current maintainer and the
much smarter person who started the project... :-)

/D
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Re: Open / Save to cloud on Windows 10

2017-02-20 Thread Linus Torvalds 
On Mon, Feb 20, 2017 at 6:49 AM, Jérémie Guichard  wrote:
>
> I narrowed it!

Good job.

> 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)...

Ugh. Ok. So it appears that whatever Windows posix emulation library
we use is pretty broken.  Looking at the "p_stat()" implementation in
libgit2, it just does a utf8->wchar expansion.

We actually do the same thing in "subsurface_open()", for all the same
reasons. I wonder why the system "stat()" routine can't just get that
right? What the hell is wrong with Windows libraries? Why doesn't the
windows "stat()" routine just DTRT?

> 3. Implement our own portable stat

I think this is the right thing to do, we already end up having all
the pieces, and we already use "fstat()" even on windows, so we could
just do a subsurface_stat() exactly like we do subsurface_open().

Linus
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Re: Open / Save to cloud on Windows 10

2017-02-20 Thread Dirk Hohndel 
On Mon, Feb 20, 2017 at 09:49:54PM +0700, Jérémie Guichard wrote:
> Hey guys,
> 
> I narrowed it!

Excellent.

> 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...

Yes, please. I don't recall ever calling curl's buldconf script, so I'm
curious what you ran into.

> 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 :)

Yes, it is somewhat heavy...

> 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)...

Cool. Very cool.

> 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.

Yeah, that one has bad design written all over it.

> 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).

Not a bad idea - but read on

> 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...

No, that seems like the wrong approach.

> 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?

How about writing a wrapper function that can be called from C in
qthelper.cpp (we have quite a few of them there already) and have that do
something like

extern "C" bool dir_exists(const char *dir)
{
return QDir(dir).exists();
}

That shoves the responsibility of platform independent implementation down
to Qt which in the past has worked very well for us. Do you think that
would work? I'm not sure if this covers all the tests we need to do, so
please expand as necessary for the two call sites.

> 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...

Excellent plan. Time zones mean that it may take a bit longer to get
answers, but I think that's usually worth it.

We support roughly the following:

Any type of Linux, with binaries being built for about 30 platforms, from
odd ARM versions to i686/x86_64 for Fedora, OpenSUSE, Ubuntu and related
OSs. Plus a generic Linux AppImage that runs on most (but not all) x86
Linux flavors. With occasional strange bugs :-)

Also, native build on Mac (because of our dependencies we now support 10.8
and newer, I believe). Native build on Mac for iOS (that's
Subsurface-mobile). Build on

Re: Open / Save to cloud on Windows 10

2017-02-20 Thread 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 :

> 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

Open / Save to cloud on Windows 10

2017-02-19 Thread Jérémie Guichard 
Hey Dirk,

Thanks for the fast reply!

Computer is Suunto Zoop. With Suunto DM5 I have to sync multiple times
before all logs are synced...

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...

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!!!

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...)

Cheers,

Jeremie


2017-02-19 14:41 GMT+07:00 Dirk Hohndel :

> On Sun, Feb 19, 2017 at 02:05:18PM +0700, Jérémie Guichard wrote:
> > Hi guys,
> >
> > First of all, I'd like to thank you all for the job well done, though
> many
> > improvements and features are to come, subsurface is quite complete and
> > useful. I could add that in my case importing logs from dive computer
> > actually works better than computer's proprietary software! (There I have
> > to sync multiple times before all my logs actually get imported while
> with
> > subsurface it works in one shot and "out of the box"...)
>
> Cool. Which dive computer is that?
>
> > I have several ideas for features and improvements (that I could help
> > coding) and am more willing to participate with translation effort, (my
> > mother tongue is French).
>
> Awesome. There are two different angles to translations: the application
> itself (I think the French translation is fairly complete) which is done
> through Transifex, and our website, where we have a slighly odd setup to
> translate things through a github repository and pull request for that
> (https://github.com/Subsurface-divelog/subsurface-website)
>
> > I'd like to deal with one issue that is pretty
> > important for me since I want to use the mobile app too, the "cloud
> > storage".
> >
> > In the current state, on my Windows 10 machine, whenever I try to: "Open
> > cloud storage" in end up with a "Error connecting to Subsurface cloud
> > storage". Same thing if I log some dives and try to "Save to cloud
> > storage". Using an Ubuntu virtual machine, the same operations succeeds.
> > (So no issue with credentials or unregistered account)
>
> I assume this is with our official 4.6.1 installer, downloaded from our
> website?
>
> > Starting the app with logging options (--win32console --verbose -v -v), I
> > get such messages (for the "Open Cloud Storage")
> >
> > git_remote_repo: accessing
> > https://cloud.subsurface-divelog.org//git/xx...@xxx.xxx
> > git storage: 0 % ( start git interaction )
> > git storage: create_local_repo
> > Cloud storage: checking connection to cloud server
> > Checking cloud connection...
> > git storage: 0 % ( waited 1 sec for cloud connetion )
> > Cloud storage: successfully checked connection to cloud server
> > git storage: 0 % ( successfully checked cloud connection )
> > git storage: calling git_clone()
> > *git storage: returned from git_clone() with error -4*
> >
> > According to libgit2 git2/errors.h this is
> >  GIT_EEXISTS = -4, /**< Object exists preventing operation */
> >
> > This lead me to navigate
> > to  C:\Users\\AppData\Roaming\Subsurface\cloudstorage and delete the
> > contained git repo.
> > At that point I can actually get my cloud stored dives locally. If I try
> > the "Open cloud storage" again the original error comes again unless I
> > delete the git repo again.
> >
> > Now if I edit or add a dive, and "Save to cloud storage", same error.
> > I delete the git repo, then "Save to cloud storage"
>
> Yikes. That's weird.
>
> > git_remote_repo: accessing
> > https://cloud.subsurface-divelog.org//git/xx...@xxx.xxx
> > git storage: 0 % ( start git interaction )
> > git storage: create_local_repo
> > Cloud storage: checking connection to cloud server
> > Checking cloud connection...
> > git storage: 0 % ( waited 1 sec for cloud connetion )
> > Cloud storage: successfully checked connection to cloud server
> > git storage: 0 % ( successfully checked cloud connection )
> > git storage: calling git_clone()
> > delete proxy setting
> > cloud certificate considered valid, forcing it valid
> > cloud certificate considered valid, forcing it valid
> > cloud certificate considered valid, forcing it valid
> > git storage: returned from git_clone() with error 0
> > git storage: do git save
> > git storage: 0 % ( start git save )
> > git storage: 0 % ( start create git tree )
> > git storage: 0 % ( start saving dives )
> > git storage: 0 % ( done creating git tree )
> > git storage, write git tree
> > existing filename
> >

Re: Open / Save to cloud on Windows 10

2017-02-19 Thread Miika Turkia 
On Sun, Feb 19, 2017 at 9:05 AM, Jérémie Guichard  wrote:
>
> In the current state, on my Windows 10 machine, whenever I try to: "Open
> cloud storage" in end up with a "Error connecting to Subsurface cloud
> storage". Same thing if I log some dives and try to "Save to cloud storage".
> Using an Ubuntu virtual machine, the same operations succeeds. (So no issue
> with credentials or unregistered account)

Just to rule something out, have you tried the following (from our FAQ):
---8<---
What to do when authentication to cloud storage fails?

If you have configured your correct user credentials to Subsurface and
cloud authentication still fails, try to change the password on
Subsurface preferences. There has been a change in the cloud service
that can cause an error on sync, but password change with correct
credentials fixes the issue.
---8<---

Your error message is not about authentication, but then again, I do
not remember what the actual exact error message was in the case where
resetting the password fixed the problem.

miika
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Re: Open / Save to cloud on Windows 10

2017-02-18 Thread Dirk Hohndel 
On Sun, Feb 19, 2017 at 02:05:18PM +0700, Jérémie Guichard wrote:
> Hi guys,
> 
> First of all, I'd like to thank you all for the job well done, though many
> improvements and features are to come, subsurface is quite complete and
> useful. I could add that in my case importing logs from dive computer
> actually works better than computer's proprietary software! (There I have
> to sync multiple times before all my logs actually get imported while with
> subsurface it works in one shot and "out of the box"...)

Cool. Which dive computer is that?

> I have several ideas for features and improvements (that I could help
> coding) and am more willing to participate with translation effort, (my
> mother tongue is French).

Awesome. There are two different angles to translations: the application
itself (I think the French translation is fairly complete) which is done
through Transifex, and our website, where we have a slighly odd setup to
translate things through a github repository and pull request for that
(https://github.com/Subsurface-divelog/subsurface-website)

> I'd like to deal with one issue that is pretty
> important for me since I want to use the mobile app too, the "cloud
> storage".
> 
> In the current state, on my Windows 10 machine, whenever I try to: "Open
> cloud storage" in end up with a "Error connecting to Subsurface cloud
> storage". Same thing if I log some dives and try to "Save to cloud
> storage". Using an Ubuntu virtual machine, the same operations succeeds.
> (So no issue with credentials or unregistered account)

I assume this is with our official 4.6.1 installer, downloaded from our
website?

> Starting the app with logging options (--win32console --verbose -v -v), I
> get such messages (for the "Open Cloud Storage")
> 
> git_remote_repo: accessing
> https://cloud.subsurface-divelog.org//git/xx...@xxx.xxx
> git storage: 0 % ( start git interaction )
> git storage: create_local_repo
> Cloud storage: checking connection to cloud server
> Checking cloud connection...
> git storage: 0 % ( waited 1 sec for cloud connetion )
> Cloud storage: successfully checked connection to cloud server
> git storage: 0 % ( successfully checked cloud connection )
> git storage: calling git_clone()
> *git storage: returned from git_clone() with error -4*
> 
> According to libgit2 git2/errors.h this is
>  GIT_EEXISTS = -4, /**< Object exists preventing operation */
> 
> This lead me to navigate
> to  C:\Users\\AppData\Roaming\Subsurface\cloudstorage and delete the
> contained git repo.
> At that point I can actually get my cloud stored dives locally. If I try
> the "Open cloud storage" again the original error comes again unless I
> delete the git repo again.
> 
> Now if I edit or add a dive, and "Save to cloud storage", same error.
> I delete the git repo, then "Save to cloud storage"

Yikes. That's weird.

> git_remote_repo: accessing
> https://cloud.subsurface-divelog.org//git/xx...@xxx.xxx
> git storage: 0 % ( start git interaction )
> git storage: create_local_repo
> Cloud storage: checking connection to cloud server
> Checking cloud connection...
> git storage: 0 % ( waited 1 sec for cloud connetion )
> Cloud storage: successfully checked connection to cloud server
> git storage: 0 % ( successfully checked cloud connection )
> git storage: calling git_clone()
> delete proxy setting
> cloud certificate considered valid, forcing it valid
> cloud certificate considered valid, forcing it valid
> cloud certificate considered valid, forcing it valid
> git storage: returned from git_clone() with error 0
> git storage: do git save
> git storage: 0 % ( start git save )
> git storage: 0 % ( start create git tree )
> git storage: 0 % ( start saving dives )
> git storage: 0 % ( done creating git tree )
> git storage, write git tree
> existing filename
> https://cloud.subsurface-divelog.org//git/xx...@xxx.xxx[xx...@xxx.xxx]
> 
> Then I open again (with a deletion of local repo in between) and nothing
> was actually saved to cloud.

Since we always first save to the local repo and then sync the repo to the
cloud, your data might have gotten lost there somehow.

> The same operations (excluding deletion of local repo) works under Ubuntu.
> 
> My best guess is that checking the presence of local repo somehow fails and
> fresh clone is always attempted (resulting in the GIT_EEXISTS error since
> the folder is already present). Further investigation would need deeper
> code investigation. This issue do not seem to be reported in github so I
> preferred to contact you before creating the ticket...

This is the first time that we have seen this type of report - and we have
several thousand users on Windows (not sure how many use the cloud
storage). I can happily load from and store to the cloud storage when
running Subsurface in a Windows VM.

Let's ask some simple questions. You have excluded your user name from the
path and replaced it with  (not unreasonable). Is that user name "all
ASCII" or are there