I ran into this a couple of times where the debug output didn't seem to
make any sense until I understood that libgit simply didn't give me
detailed error info.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This is not a great way to load-balance, but it works and doesn't require
high end hardware on the backend.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
We had various random "free parts of the git info" left-overs from when
we passed down the git repo data ad-hoc. Get rid of it, and replace it
with just doing a 'cleanup_git_info()' that does the final cleanup of it
all.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
We currently only have one single caller of update_local_repo(), and
instead of that caller checking whether the existing repo is a
directory, just make it open the git repository.
This avoids duplicate error handling and simplifies the code.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Just like the rest of the git repo related information, this is already
included in the git_info struct.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
We have this nasty habit of randomly passing down all the different
things that we use to look up the local and remote git repository, and
the information associated with it.
Start collecting the data into a 'struct git_info' instead, so that it
is easier to manage, and easier and more logical to just look up
different parts of the puzzle.
This is a fairly mechanical conversion, but has moved all the basic
information collection to the 'is_git_repository()' function. That
function no longer actually opens the repository (so the 'dry_run'
argument is gone, and instead a successful 'is_git_repository()' is
followed by 'opn_git_repository()' if you actually want the old
non-dry_run semantics.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
If we can't reach the cloud server in the URL (which might come from the
settings or be passed in by the user), we try the alternative server(s).
If we end up changing servers, we need to update the remote that we have
already parsed from the URL.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
If we can't reach our preferred server, try using a different one.
The diff makes more sense when ignoring white space.
With this we check the connection to the cloud server much earlier and
in case of failure to connect try a different cloud_base_url.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
With the new names for the cloud server we'd get different local cache
directory names depending on which server gets used. In order to avoid
that, normalize the name before generating the hash that determines the
local directory name.
Additionally, the old code had an extra '/' in the URL, due to the way
the URL was assembled. Again, to match the existing hash for people
upgrading from older Subsurface versions, add that to our normalized
name as well.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
The backend infrastructure will soon be able to support more than one
cloud server which automagically stay in sync with each other.
One critical requirement for that to work is that once a session was
started with one of the servers, the complete session happens with that
server - we must not switch from server to server while doing a git
transaction. To make sure that's the case, we aren't trying to use DNS
tricks to make this load balancing scheme work, but instead try to
determine at program start which server is the best one to use.
Right now this is super simplistic. Two servers, one in the US, one in
Europe. By default we use the European server (most of our users appear
to be in Europe), but if we can figure out that the client is actually
in the Americas, use the US server. We might improve that heuristic over
time, but as a first attempt it seems not entirely bogus.
The way this is implemented is a simple combination of two free
webservices that together appear to give us a very reliable estimate
which continent the user is located on.
api.ipify.org gives us our external IP address
ip-api.com gives us the continent that IP address is on
If any of this fails or takes too long to respond, we simply ignore it
since either server will work. One oddity is that if we decide to change
servers we only change the settings that are stored on disk, not the
runtime preferences. This goes back to the comment above that we have to
avoid changing servers in mid sync.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
We know the preference is never empty, so stop testing for this. But
don't maintain two different preferences with basically the same
content. Instead add the '/git' suffix where needed and keep this all in
one place.
Simplify the extraction of the branch name from the cloud URL.
Also a typo fix and a new comment.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
gcc complained about two constructs of the kind
remote_id && SSRF_INFO("...");
And while I am not a fan of excessive warnings, I must say
it has a point here. That's just code obfuscation. In fact,
it appears that the condition was wrong - the SSRF_INFO
should probably be invoked if remote_id is NULL. The way
it was written it would be invoked if it was *not* NULL.
Change both instances to unfancy if statements.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This may seem like a bit heavy handed as it adds more global state, but given
the number of ways in which attempts to sync with the cloud can fail it seems
much more reliable to claim success in the spots where we actually know that we
have successfully synced with the remote server. Transporting that information
back through the various call chains turned out to be very disruptive and ugly,
so I went with global state instead.
Whenever we access cloud storage (or any git repo), we always first check if it
actually is a git repo by calling is_git_repository() - so this is the perfect
spot to initialize the variable to false.
And there are only two spots where we either clone the remote repo
(create_local_repo()) or update the remote with the (potentially merged) local
changes (check_remote_status()). So those are the two places where we set the
variable to true.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
In many cases we did not log the issues the code ran into to stderr which made
remote debugging user problems much harder. This hopefully will help with that.
Since I was looking at the code, I also made the existing messages more
consistent.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Turns out that at least on Android libgit2 sometimes rejects valid
certificates. And I cannot quite figure out when and why. But since we
actually already checked the validity of the certificate when we called
canReachCloudServer() (and the Qt code handles certificates correctly),
we'll simply ignore this here and override the check to always return
true for our cloud server.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
I would have bet money that Android used to send stderr to the logcat
log, but apparently it doesn't (anymore?). So in order to be able to
have a chance to debug weird cloud storage issues on Android, let's do
some wholesale replacement of fprintf(stderr,...) with our own version
of the INFO macro that we long ago borrowed from libdivecomputer (and
rename it to ensure we don't have a conflict there).
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This will be used by the test to clean up test branches that are created
on the server. Since we aren't testing that functionality (it's not
something that Subsurface itself ever does) the helper prints out errors
it encounters, but doesn't report them back to the caller.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
If you had one of the unfortunate local git caches with a local HEAD
just pointing to 'master', this will make note of that and then fix it
up to use the proper branch name in the cache repository.
[Dirk Hohndel: demoted from error to fprintf as most users won't care]
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
In create_and_push_remote(), we set up the remote tracking etc to use
the proper branch name, but never actually set up the initial local
branch for the new cache repository at all. So the repository would end
up with the default 'master' branch, instead of the branch name it
should have.
This went unnoticed, because most setups start by initializing the git
caches by cloning from the cloud, and that worked fine.
Debugged-by: Dirk Hohndel <dirk@hohndel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This never made sense and I think I just forgot to complete this code
when I first worked on it. Now we can see which version of Subsurface or
Subsurface-mobile created a merge.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
A number of architecture-dependent functions were declared in
dive.h. Move them to file.h so that not all file-manipulating
translation units have to include dive.h. This is a small step
in avoiding mass-recompilation on every change to dive.h
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Move the declarations of the "report_error()" and "set_error_cb()"
functions and the "verbose" variable to errorhelper.h.
Thus, error-reporting translation units don't have to import the
big dive.h header file.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This is only used by one caller and there doesn't appear to be a reason
to inline it in the first place.
Suggested-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Since all qt-helpers are defined in qthelper.cpp, there seems to be
no reason to have two include files. By unifying the two files,
duplication and inconsistencies are removed. The C++-only part is
simply compiled away with #ifdefs.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Mostly replace "return (expression);" by "return expression;" and one
case of "function((parameter))" by "function(parameter)".
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The username was extracted from https:// urls but not from ssh://
urls. Unify this by extracting the username from any remote url.
This is done with regard to unifying the file handling in the
frontend.
For this approach to work, the credential callback of the ssh://
transport had to be adapted. It now also supports username/password
in addition to private-key authentication.
Currently, the only way the user can use the username/password
authentication is by deleting a potential public key.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
On saving to a remote git repository, the transport was set to https://,
which broke saving to ssh:// repositories. Instead determine the
transport from the remote url.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Since this helper-function exists, we might just use it. A subtle
reuse of a buffer (string of second use was known to be shorter than
string of first use) was replaced by a separate allocation.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The libgit2 functions git_cred_ssh_key_new() and git_cred_userpass_plaintext_new()
copy their arguments. Therefore, free the string arguments or don't
copy them in the first place.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
In case syncing with the online repository failed, enter offline mode.
This reflects the message sent to the user ("working with local copy").
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
If the credential functions return GIT_EUSER, a call to git_remote_fetch
fails, but giterr_last() may return NULL. This led to a crash in
verbose mode.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
In this function, a repository is created, but the returned object
is not used. Might just as well free it.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Currently, in is_remote_git_repository(), git URLs of the form
"file://..." are recognized as local and the "file://" prefix is
removed. The shortened URL is then processed as if it was a remote
URL, which of course has to fail. So far so good - this is not
a remote repository after all. But the removal of the prefix is
not propagated to the calling is_git_repository() function and
handling as a local git repository therefore fails likewise.
To fix this issue, move removal of the "file://" prefix one level
up to the is_git_repository() function.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
