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

Release 3.3.0 #802

Closed
17 of 18 tasks
kelson42 opened this issue Jan 15, 2022 · 70 comments
Closed
17 of 18 tasks

Release 3.3.0 #802

kelson42 opened this issue Jan 15, 2022 · 70 comments
Assignees
Labels
build Code relating to building, publishing, or maintaining the app information tests
Milestone

Comments

@kelson42
Copy link
Collaborator

kelson42 commented Jan 15, 2022

The 3.3 milestone is already quite interesting and has still a lot of work to do. I see work to do for at least 4 months. Last release is already almost 6 months ago. Release early, release often is good. I want to encourage you to reassess the milestone to 3.3.0.

@kelson42 kelson42 added question build Code relating to building, publishing, or maintaining the app labels Jan 15, 2022
@kelson42 kelson42 added this to the v3.3 milestone Jan 15, 2022
@Jaifroid
Copy link
Member

@kelson42 - [I fixed tick boxes above]

I agree we are quite close. We don't need to do everything marked as v3.3, we can push those not complete and those that are not essential to v3.4. I'll add the current PRs we should merge to this.

@mossroy
Copy link
Contributor

mossroy commented Jan 15, 2022

I fixed the version numbers in the tick boxes.

I agree we're close to have enough changes to release a 3.3

@Jaifroid
Copy link
Member

See #805 for initial changelog.

@mossroy
Copy link
Contributor

mossroy commented Feb 2, 2022

I suggest that we try to do this release this week-end, if possible.
If the current open PRs are not merged, they will be for next release, never mind

@mossroy
Copy link
Contributor

mossroy commented Feb 4, 2022

I've done some tests on my Firefox OS device, with various ZIM files.
Apart from #814 , I did not find any issue.

@Jaifroid
Copy link
Member

Jaifroid commented Feb 5, 2022

@mossroy I've just updated the implementation at moz-extension.kiwix.org to be inline with current master. This will facilitate testing. I'd like to do a bit more testing of the switch between JQuery and SW in Firefox extension, as this is the headline feature of this release... I propose to complete these tests today, so we can create the tag tomorrow.

What should we do about the Ubuntu phone release? Cross fingers and hope? I'm not even sure ernesst is using that version any more!

@mossroy
Copy link
Contributor

mossroy commented Feb 5, 2022

Thanks.
Yes, I agree, I'm currently testing too.

Regarding Ubuntu phones, I think we can not do anything more than "cross fingers", until we have another tester.
But I think we should still release and deploy for this device. Unless they changed the underlying browser engine (which seemed to be a not-so-old Chromium), I think it's unlikely that a regression would happen on this platform, and we never heard of any regression so far.
In any case, there are 2 possibilities:

  • new releases work well on Ubuntu phones, and their users are happy
  • there are regressions and hopefully someone will report them here, and would become a tester

@cibersheep
Copy link
Contributor

Regarding Ubuntu phones, [...]

Hello, just passing by :)
I can test for Ubuntu Touch. If you need more testers, the forum of the community is a great place to announce

Now that I'm here, I cannot seem to find any click. Do you have a link or do I have to build the project?

@Jaifroid
Copy link
Member

Jaifroid commented Feb 5, 2022

@cibersheep Thank you, amazing you saw this!

There's a click here: http://download.kiwix.org/nightly/2022-02-05/ . The only thing is that there was a merge this morning, so it might be good to test also after tonight's is generated. If you're able to test, that would be great.

@cibersheep
Copy link
Contributor

cibersheep commented Feb 5, 2022

Ah! Awesome. Thanks. I can do that ;)

Edit: I can also do this: https://forums.ubports.com/topic/7390/kiwik-ja-needs-testers

^__^

@cibersheep
Copy link
Contributor

cibersheep commented Feb 5, 2022

I'm testing on Pixel 3a latest dev:
Tested: kiwix-ubuntu-touch-3.3commit-b143e83.click

  • Install click: ok
  • API warning: ServiceWorker API available but not registered
  • API error: Error loading ZSTD descompressor
  • Load a zim file (~140 MB): ok
  • Search finds results: ok
  • Tapping on a result shows loading icon

log: https://dpaste.com/6HZUX2QED

@Jaifroid
Copy link
Member

Jaifroid commented Feb 5, 2022

@cibersheep Thank you. So you get the spinner but it never stops, and doesn't show an actual article?
For comparison, I wonder if you could check the same ZIM archive by visiting the test implementation here in the device's browser:

https://kiwix.github.io/kiwix-js/

@Jaifroid
Copy link
Member

Jaifroid commented Feb 5, 2022

So according to the log, the required decoder (ZSTD) is not loading, as we get Could not instantiate any ZSTD decoder! TypeError: Failed to fetch file:///usr/share/morph-browser/ContentPickerDialog.qml:118: ReferenceError: acceptTypes is not defined.

I haven't seen a need for acceptTypes before. Could this be a framework-specific error? Maybe a packaging error as we probably haven't updated the click packager with all the files or file types required? I'm guessing a bit here, though the log is very useful.

EDIT: I'm not sure why the ContentPickerDialog is throwing an error, but as it seems you were able to pick the ZIM archive and even search it, then I assume that error is immaterial. The more serious error is not loading the decoder. We have two decoders, XZ for older ZIM archives, and ZSTD for newer ones (last 18 months or so). We only instantiate the one needed for a specific ZIM archive. There is an XZ-encoded split ZIM in the tests directory. It would be very useful to find out if you can open and see articles from that ZIM. You need to pick ALL the parts in the file picker. This will tell us if the problem is specific to the ZSTD decoder or not.

Searching for results does not require the decoder, but accessing an article does.

@mossroy
Copy link
Contributor

mossroy commented Feb 5, 2022

@cibersheep : great to have you here to test on Ubuntu Touch!
Could you install some older versions of kiwix-js, to see if they are better on your device/software? https://open-store.io/app/kiwix/versions

When we have more information, we should open a separate ticket for that.

@cibersheep
Copy link
Contributor

cibersheep commented Feb 5, 2022

@cibersheep Thank you. So you get the spinner but it never stops, and doesn't show an actual article? For comparison, I wonder if you could check the same ZIM archive by visiting the test implementation here in the device's browser:

https://kiwix.github.io/kiwix-js/

This was a lot faster! Nice.

  • Loaded zim file
  • It showed some articles
  • Search worked as in the app
  • Selecting an article showed it quite fast
    log (Morph): https://dpaste.com/8J9WEX8SP

So according to the log, the required decoder (ZSTD) is not loading, as we get Could not instantiate any ZSTD decoder! TypeError: Failed to fetch file:///usr/share/morph-browser/ContentPickerDialog.qml:118: ReferenceError: acceptTypes is not defined.

I haven't seen a need for acceptTypes before. Could this be a framework-specific error? Maybe a packaging error as we probably haven't updated the click packager with all the files or file types required? I'm guessing a bit here, though the log is very useful.

I think TypeError is an error for the toolkit. Nothing specific for Kiwix app

EDIT: I'm not sure why the ContentPickerDialog is throwing an error, but as it seems you were able to pick the ZIM archive and even search it, then I assume that error is immaterial.

Is a known error. It shouldn't prevent the Picker to load the file. As I'm able to search for articles, I assume they are loaded correctly (?)

The more serious error is not loading the decoder. We have two decoders, XZ for older ZIM archives, and ZSTD for newer ones (last 18 months or so). We only instantiate the one needed for a specific ZIM archive. There is an XZ-encoded split ZIM in the tests directory. It would be very useful to find out if you can open and see articles from that ZIM. You need to pick ALL the parts in the file picker. This will tell us if the problem is specific to the ZSTD decoder or not.

Searching for results does not require the decoder, but accessing an article does.

When opening the app for second time:

  • JS error is more useful[1]
  • zim file is not present

[1]
imatge

@cibersheep : great to have you here to test on Ubuntu Touch! Could you install some older versions of kiwix-js, to see if they are better on your device/software? https://open-store.io/app/kiwix/versions

When we have more information, we should open a separate ticket for that.

Mmm v3.2.0 seems not to load the zim file at all. Decoder error is the same. I'll test older versions in a sec

Version 3.1.0 works properly (loads zim file, search and article shows). Decoder error is not present (aka it loads)

@Jaifroid
Copy link
Member

Jaifroid commented Feb 5, 2022

@cibersheep Yes, the fact you can pick the ZIM and search for articles (and get a list of articles matching the search) shows that the file picker error is immaterial.

The issue is the way the decoder is failing to load. It's strange, because the decoder loads in IE11 when accessing from the file:// protocol, so it can't be an issue with the fact that the Ubuntu touch framework uses file://

The warning dialogue box above: this should actually disappear with tonight's nightly, because this morning's merge was precisely a fix for a bug that makes incompatible frameworks attempt to switch to Service Worker mode. The message shown is actually harmless, it's just telling you that the app tried (erroneously) to switch to using a Service Worker when that is not possible over the file:// protocol.

The fact that the test implementation works in the device's browser shows that there is no fundamental incompatibility in the code, which is good. It also means that it should be possible to use this app as an offline-first PWA if all else fails (and if Service Worker mode is supported when accessing the https:// implementation).

However, the main task is to find out why 3.1.0 works, when the later ones don't.

@Jaifroid
Copy link
Member

Jaifroid commented Feb 5, 2022

OK, it's undoubtedly this change in v3.2.0:

  • NEW: Use fast binary WASM decoders with fallback to ASM if necessary

I think loading WASM is failing, and the fallback option is not being detected. I don't know exactly why. It works in Chromium extensions which also use the file:// protocol and a Chromium browser, so there is no obvious reason why it's failing here.

As a quick fix (monkey wrench.......) we could explicitly load the ASM versions (which are actually just as fast, btw) if we can detect that this will fail. I think we do something like that for IE11. But this should probably be a new issue. I'll see if I can move these comments.

@mossroy
Copy link
Contributor

mossroy commented Feb 6, 2022

You may have noticed the PWA is set to 3.3 instead of 3.3.0

Yes, but it's probably not a big issue. Don't bother.

@mossroy
Copy link
Contributor

mossroy commented Feb 6, 2022

@kelson42
Copy link
Collaborator Author

kelson42 commented Feb 6, 2022

@mossroy Submitted to review, should be published until tomorrow.

@Jaifroid
Copy link
Member

Jaifroid commented Feb 6, 2022

OK, it looks like the failure to update the extension PWA wasn't a problem with my workflow scripting, but a failure on the docker side. Here is the workflow:

https://github.com/kiwix/kiwix-js/runs/5084057842?check_suite_focus=true

I'd forgotten that I had in fact written code to modify the version number in the source files, and the new number is indeed set by the tag.

image

We do know that the scripting works here, so long as VERSION was in fact set to TAG_VERSION (because we use the same scripting to set the version from the UI). Let me know if you think I need to change anything.

@mossroy
Copy link
Contributor

mossroy commented Feb 8, 2022

I'm wondering if it could not come from the fact that our tags are named "3.3.0", and not "v3.3.0" (which is more common).
Maybe a script removes the first character of the tag (assuming it's always the leading "v")

@mossroy
Copy link
Contributor

mossroy commented Feb 8, 2022

If we want to release a fix-only version 3.3.1, we might merge #818 and #827, then release a 3.3.1, before merging any other PR

@Jaifroid
Copy link
Member

Jaifroid commented Feb 8, 2022

I'm wondering if it could not come from the fact that our tags are named "3.3.0", and not "v3.3.0" (which is more common). Maybe a script removes the first character of the tag (assuming it's always the leading "v")

Well here is where we set the tag-pattern that the job is supposed to use:

image

https://github.com/kiwix/kiwix-js/blob/master/.github/workflows/publish-extension.yaml#L67

And remember that the job runs correctly when we set a tag from the Action Dispatch UI, without a v. We may just have been unlucky.

What I would like to do is to add some echo statements so that if it fails again we get a better log of what values we are passing. We can also test the logging before 3.3.1 because if we push master again as 3.3, there will be no change for users. I'll make a PR for logging (in the Workflow).

@Jaifroid
Copy link
Member

Jaifroid commented Feb 8, 2022

But it is possible that there is a coding issue upstream in the GitHub action, which somehow ignores the tag-pattern, but doesn't do any processing on the manual-tag.

@Jaifroid
Copy link
Member

Jaifroid commented Feb 8, 2022

@mossroy Will 3.3.1 need a changelog with the two Ubuntu Touch updates?

@mossroy
Copy link
Contributor

mossroy commented Feb 8, 2022

We should indeed add this version 3.3.1 to the changelog file

@mossroy
Copy link
Contributor

mossroy commented Feb 9, 2022

When #829 will be merged, we could merge #830 and release version 3.3.1 for all targets (not only for Ubuntu Touch)

@mossroy
Copy link
Contributor

mossroy commented Feb 12, 2022

I've done some basic tests on Firefox and Chromium (including as extensions), and on Firefox OS. Found no regression (risk is very low based on what has changed from 3.3.0)
I'm about to create the 3.3.1 tag.

In my opinion, we should deploy this version 3.3.1 for all platforms. The race condition might have affected some other devices we did not hear about.
The packages will be automatically generated, we only have to deploy them on the stores.

@Jaifroid
Copy link
Member

OK, fine by me! I tested on IE11 in case that's useful, and saw no regression. On IE11 it now shows the decompressors as uninitialized in the API panel, until a ZIM is loaded, at which point it populates the panel. This is simply to do with the fallback to ASM, as the WASMs get initialized (loaded) by the script, whereas ASM is just plain JS (albeit optimized). Everything works well on IE11.

@Jaifroid
Copy link
Member

This is what I mean:
IE11:
image

Edge:
image

@mossroy
Copy link
Contributor

mossroy commented Feb 12, 2022

OK that's fine.
I've just created tag 3.3.1, and it generated the packages in https://download.kiwix.org/nightly/2022-02-12/
I'll upload for Firefox and Ubuntu Touch

@Jaifroid
Copy link
Member

Yay, and the workflow correctly updated the PWA. Phew!

image

image

@mossroy
Copy link
Contributor

mossroy commented Feb 12, 2022

Cool for the PWA! Nice job
The 3.3.1 version is uploaded for Ubuntu Touch. @cibersheep could you check that it works well? https://open-store.io/app/kiwix
It's also available on Firefox, and its PWA has been properly updated to 3.3.1 on mine. Cool!
@kelson42 , could you upload https://download.kiwix.org/nightly/2022-02-12/kiwix-chrome-unsigned-extension-3.3.1.zip for Chrome?

@mossroy
Copy link
Contributor

mossroy commented Feb 12, 2022

I've unchecked what needs to be done again for 3.3.1 in the checklist of this ticket

@Jaifroid
Copy link
Member

3.3.1 uploaded to Edge store.

@kelson42
Copy link
Collaborator Author

3.3.1 uploaded to Chrome store.

@kelson42
Copy link
Collaborator Author

Download.kiwix.org updated with the 3 versions

@cibersheep
Copy link
Contributor

cibersheep commented Feb 12, 2022

Cool for the PWA! Nice job The 3.3.1 version is uploaded for Ubuntu Touch. @cibersheep could you check that it works well? https://open-store.io/app/kiwix It's also available on Firefox, and its PWA has been properly updated to 3.3.1 on mine. Cool! @kelson42 , could you upload https://download.kiwix.org/nightly/2022-02-12/kiwix-chrome-unsigned-extension-3.3.1.zip for Chrome?

I'm afraid not: qml: [JS] (file:///opt/click.ubuntu.com/kiwix/3.3.1/www/js/lib/zstddec_wrapper.js:115) WASM failed to load, falling back to ASM... TypeError: Failed to fetch *** Error in 'webapp-container': corrupted double-linked list: 0x000000001daaa6d0 ***

I send you a couple of logs (they might be a bit useless):

Update from test app to 3.3.1 and tried to open local zim
https://dpaste.com/FX9YH9BWE

Cleared cache:
https://dpaste.com/EHD9JRBF5

Cleared cache, uninstall, reboot phone, reinstall
https://dpaste.com/CJJWVVNQQ

Reset app from in app
https://dpaste.com/2R6LSV7C9

@Jaifroid
Copy link
Member

@cibersheep The failure to load WASM is expected: it is supposed to fall back gracefully to ASM, and that is what was happening with the nightly, right? However, it shouldn't then error out and fail to load/display a ZIM (if I've understood correctly what is happening). Could you kindly check the last nightly that works? From http://download.kiwix.org/nightly/ . I don't see what can have changed between the last nightly that was working and release... But hopefully we can get to the bottom of it.

@Jaifroid
Copy link
Member

I wonder if these lines are significant errors caused by AppArmor policies? I'm trying to figure out if it's anything to do with APIs that are disallowed by the store... (but that run in a non-store click package)?

image

@cibersheep
Copy link
Contributor

I wonder if these lines are significant errors caused by AppArmor policies? I'm trying to figure out if it's anything to do with APIs that are disallowed by the store... (but that run in a non-store click package)?

image

The apparmor polices are how the app works on the phone itself. That have been caught but the test app as well. I'm prety sure you can ignore them

@cibersheep
Copy link
Contributor

cibersheep commented Feb 12, 2022

Ok. I've been doing some tests. I have bad and good news:

  • Non of the last versions works on my main device
  • 3.3.1 works on my tablet (I had no previous versions installed before)

I'm guessing is a change in the OS of my phone?
I will ask more testers

For reference:
Pixel 3a none worked. Ustack before unpaking the zim. Between installs I've cleared cache and desintalled
http://download.kiwix.org/nightly/2022-02-12/kiwix-ubuntu-touch-3.3commit-374d184.click
https://dpaste.com/5TCE3H5Y6

http://download.kiwix.org/nightly/2022-02-11/kiwix-ubuntu-touch-3.3commit-829aaf2.click
https://dpaste.com/3HKDUDQFS

http://download.kiwix.org/nightly/2022-02-10/kiwix-ubuntu-touch-3.3commit-829aaf2.click
https://dpaste.com/3SFCA4ALL

http://download.kiwix.org/nightly/2022-02-09/kiwix-ubuntu-touch-3.3commit-a318835.click
https://dpaste.com/59YUARYTK

With this version I redownloaded the zim file (I wanted to check if it was corrupted)
http://download.kiwix.org/nightly/2022-02-08/kiwix-ubuntu-touch-3.3commit-ba994b8.click
https://dpaste.com/AFUFGM4JX

M10HD
Worked with OpenStore version 3.3.1
https://dpaste.com/H2AURQ8VH

@Jaifroid
Copy link
Member

I've created #839 to work on this. Let's continue discussion there.

@Jaifroid
Copy link
Member

We can probably close this thread. @kelson42 will you be able to do some publicity for v3.3.1?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Code relating to building, publishing, or maintaining the app information tests
Projects
None yet
Development

No branches or pull requests

4 participants