Folder creation broken in Android app

Hi there.

I’m so sorry the transition to the new folder type supporting v14.49 syncthing version broke the part of new folder creation on the android app. I’ve immediately put up a PR to fix the problem, so please hold on and don’t report this bug again. Link to github ( https://github.com/syncthing/syncthing-android/pull/1191 )

Until the fix is released, you can work around this by toggling the “Send Only” slider “back and forth” before saving a folder you’d like to create.

Changing existing folders still works properly.

Kind regards Catfriend1

EDIT: 389 (and counting) sessions were affected. Feedback on GPlay confirmed the bug.

1 Like

Adding tests with/for new features and bugfixes is nice. I doubt anyone expects you to produce tests for all legacy stuff. Generally, few people here should throw any stones in the testing glasshouse…

The Syncthing integration tests are unfortunately of limited value, despite efforts to clean them up. Unit tests, and hence unit testable code, is where the bang for the buck is.

(Not my project, not trying to tell you what to do, just an opinion.)

Hello there,

I am following the improvements of syncthing-app for a while already. I hope this is the correct thread to post this as discussions are spread.

What I currently think is reality: There is two people working on the syncthing wrapper. Catfriend and Audrius. Impression I gained from Audrius so far: He pushes the merge buttons and also combines translations and commits from Catfriend to the google play version. Often enough translation for the latest versions are missing as they are pushed too early to the live version. He does not seem to test the app before he pushes it to the live version. Often enough his responses to others could be improved. However, we all know he is also helping calmh to produce the real syncthing. So he must have a lot on his desk already and working on this wrapper is voluntary, so it is fine to not “manage” it more then he wants to. But I think some help for him would be welcomed. Impression I gained from Catfriend so far: With him joining the project, the app gained a lot more stability and improvements. He is not shy to acknowledge when he is not sure how something is coded, from his first baby steps in android-app world, he evolved greatly and I think it is save to say, that he does not make beginngers-fails anymore. So far I have seen him being responsible and responsive to all the problems (not only the ones he produced, but also stuff that was around for a long time already), besides that he tests his stuff and always makes a note until which point he tested, so actually it is easy to follow. However, his reach is limited to the github world, so not many people on google play will ever see his documentation. This gets even clearer by recent events. Let’s get to this now.

There is currently three topics going on:

  1. Hotfixes should be applied as soon as they are ready to a live version that has a major problem
  2. QA is week as there is not enough developers, so a solution is needed
  3. Management of PRs, QA and Release could be improved.

Let’s elaborate on them a bit more:

  1. If it got clear, that the live version has a problem, and the hotfix is simple and can be tested quickly. Then it should be pushed to the live version on all platforms with a proper announcement. Google Play seems to be having an issue in this regard. It’s just a guess but with all those android users it might be save to assume that a lot of people get the app via google play, that also means that when issues get introduced to the live version you guys lose people on Google Play, and then syncthing reputation suffers from this. -> The latest hotfix is available quite a time on github however it is not yet pushed to google play, I see no reason for this delay (however I do not know how much effort it is to push the app to google). Audrius and Catfriend seem to be both aware of the hotfix, however Catfriend seems to be not able to push it to google while Audrius might have other things to do right now.
  2. You cannot assume that one person � even if it is the programmer � can find all the issues, not even by a testing routine. Big companies have a QA-department for this. As this team is too small I am sure there is currently no way for this, besides that, it is quite time consuming. However, I see users being responsive if asked for describing the issue and even helping out to find the bug. I think it was already mentioned somewhere, but here again: Google Play has a BETA functionality, where registered users can help out testing new features and commits. But those pushes should be done several days before a version goes live to the “all user branch”. This leads me to my third point.
  3. Management of when new features go into the live version should be improved. I don’t want to elaborate on all the issues that the project currently has, in terms of new features, translations and bugs. I would like to point to a solution that could be done easily and should be tested if it is found valuable by you guys. It is quite in line with what I wrote previously and I am sure you could already guess it. And you guessed right: Use the Users. use the Beta branch on google play. As one person cannot always be available to push versions to Google (Live or Beta) at least two persons should have the rights to do so. For every feature or fix, the Beta branch should be used SEVERAL DAYS before it goes to the live branch. Be communicative on Google play and write a short text of “what has been improved”, “known issues” and maybe even “soon to come features”. I am not sure if Catfriend would like to have the responsibility and would be kind enough to help audrius out on those tasks. But I see that it would greatly help the reputation of syncthing and the app.

Thanks for reading this far. Sturmlicht

The (any project’s) release process needs streamlining and automation. I’ve done a lot of that for Syncthing but there’s still a number of steps, button presses, scripts, and commands each of which sometimes goes wrong and requires hands on to resolve. So I don’t know how this is on the Android side, but I can see how inducting a new release manager can be tricky, and two releases (beta + stable) might be twice as much work as one.

That said, it would seem like the ST-Android project could use a similar approach as ST itself - perhaps release a beta together with the corresponding ST release candidate, and then make that the production build with the corresponding ST release a month later. That’d catch breakage from API changes etc a bit earlier in the beta at least.

1 Like

Sure the release process can be improved, but I don’t want to be in the boat of having to be there every time calmh releases a syncthing rc. If we can automate rolling to beta everytime syncthing releases and rc, and then at some point releasing that (and not whats on mastern like we do now) to prod, would probably be better, but its still shit, because there are barely any users on beta track, and I don’t expect them to keep opening the app, adding folders and what not on a casual basis, as this essentially a setup once and forget app.

So fuckups will still slip through, which could pontentially be caught by unit tests.

I don’t use the app, nor do I use syncthing, so I am in no way going to be on the manual testing path.

I have not released a hotfix because I am waiting for more dead bodies to come to the surface, once we know we got all of them, I’ll make a release, probably this evening. I don’t want to be releasing hotfixes every day.

It sounds like it may be worth it to take the effort to hand over the baton for day to day releases, with as much as possible automated of course.

I do nothing useful on the Android side, I just provide opinions - for free.

This is not a significant enough issue that this needs a beta.

Your fix fixed symbols from one language, there are other languages that use non ascii characters on the planet, so it’s more like a workaround for languages that use cyrillic only, and all other people are left in the water.

Yeah, I guess I could have skipped it all together, but because we were addressing immediate reports by users, it made sense to have it part of the same boat.

Yes, I probably should have added the return at the top, but we’re past that point now, and I can’t change that.

The rm is not recursive nor it’s a force, and always ends with .stwritetest, so this “deletes everything at root” sounds implausible, as it would not match anything sensible.

I am all up for utilising the beta channel, but as I said, I don’t want to have to push a button every Tuesday, so some automation has to be developed.

So your changes are in master. As I said, I don’t think it’s a problem worthy yet another release, hence can wait out the a whole cycle.

I’ve raised some other points about passing unescaped user input straight into the root shell. This is not related to this issue, but it is an issue/oversight in general. We should either use some shell escaping utility function or back out subshelling user input.

I’ve enabled “no pushes to master unless merging a PR” and all PRs now require a review before they can be merged.

I guess we could create a release branch which essentially follows some commit on the master tree (so not actually a branch, just a marker), and every time the release branch moves, it rolls out a new beta.

Then once the beta has been stable and has not moved for atleast a week, we could promote it in gplay.

I don’t think the buffer reader fixes it, it potentially gets interpreted weirdly by the shell, but because shell output is discarded it’s hard to say how it gets interpreted.

I think wrapping the path in https://source.android.com/reference/tradefed/com/android/tradefed/util/StringEscapeUtils.html#escapeShell(java.lang.String) should be enough.

I am not saying this is a matter of life or death that needs to be fixed now. All I am saying is that as it stands, it has problems. You disagreed, and I am just trying to provide evidence to back my claim. It’s not a personal attack of some sorts.

Pull request reviews apply to everyone, and I enabled them to have the same setup like we have in syncthing, where each change, including my own, would go through a second pair of eyes. There will be unreviewed pushes, but that should only apply to making a release, or an urgent (even if it’s a shitty) change like today which allowed to fix a minor bug with the new release.

1 Like

You guys seem to be at an all-hands-on-deck firefighting situation at the moment. That’s a frustrating place to be and makes delays due to review and release cycles feel extra burdensome. Nonetheless review is a good thing and maybe extra necessary when there’s already a feeling of panic.

I’m 100% certain that Audrius turning on “review required” is not about punishing anyone or preventing fixes from getting in. It’s just good practice that should have been in place already.

Note too that it does not mean depending on a repo admin to get things merged. What you need is two people with push access to the repo - one author and one reviewer. Anyone of the 35 current members in the Syncthing org can be either of those.

I’m also quite certain that maintaining a fork is less fun than you might imagine it being.

This topic was automatically closed 30 days after the last reply. New replies are no longer allowed.