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

Add date to git-rev version #3723

Merged
merged 1 commit into from Mar 7, 2018

Conversation

Projects
None yet
2 participants
@Frenzie
Member

Frenzie commented Mar 5, 2018

Adds an extra part at the end of the git-rev.

Before: blablabla-21-g076bf406
After: blablabla-21-g076bf406_2018-03-05

Fixes #2896

screenshot_2018-03-05_21-24-12

Add date to git-rev version
Adds an extra part at the end of the git-rev.

Before: `blablabla-21-g076bf406`
After: `blablabla-21-g076bf406_2018-03-05`

Fixes #2896
@poire-z

This comment has been minimized.

Contributor

poire-z commented Mar 5, 2018

OK, nothing seems to use it (except wikipedia.lua, to put it into <dc:publisher>, which will be fine).

A little bit related: it would be nice to have it in crash.log, after our ascii art logo, so we wouldn't need to guess or ask which version a reporter is using.

@Frenzie

This comment has been minimized.

Member

Frenzie commented Mar 5, 2018

The main use is in OTA updates for which I wrote https://github.com/koreader/koreader/blob/e7f705bf1095145d522234a357269e60e868e79a/frontend/version.lua It's covered by unit tests so we should be good. https://github.com/koreader/koreader/blob/e7f705bf1095145d522234a357269e60e868e79a/spec/unit/version_spec.lua

local local_ok, local_version = pcall(function()
local rev = Version:getCurrentRevision()
if rev then return Version:getNormalizedVersion(rev) end
end)
local ota_ok, ota_version = pcall(function()
return Version:getNormalizedVersion(ota_package)
end)
-- return ota package version if package on OTA server has version
-- larger than the local package version
if local_ok and ota_ok and ota_version and local_version and
ota_version ~= local_version then
return ota_version, local_version
elseif ota_version and ota_version == local_version then
return 0
end

If anything is using the git-rev directly it should probably be refactored to use Version:getCurrentRevision() instead. Currently it simply returns the full contents of the file, so it would not technically a git-rev anymore though I'm not sure whether that matters.

The stuff used doesn't care what comes after the git revision. We could extend Version with a Version:getDate() if desired.

Visually it looks slightly better like blablabla-21-g076bf406 (2018-03-05) but spaces and parentheses are more annoying to deal with in URLs and filenames. I considered a separate git-date file or something like that, but it seemed useful to make it part of the package filename.

Frenzie added a commit to Frenzie/koreader that referenced this pull request Mar 5, 2018

Frenzie added a commit that referenced this pull request Mar 5, 2018

@Frenzie Frenzie added the enhancement label Mar 7, 2018

@Frenzie Frenzie merged commit 1606137 into koreader:master Mar 7, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@Frenzie Frenzie deleted the Frenzie:date-gitrev branch Mar 7, 2018

@poire-z

This comment has been minimized.

Contributor

poire-z commented Mar 9, 2018

I think this got some effect with the nightly builds not being availble in the final directory. The builds must be ok, but the script don't like the artifacts zip:

Fetch build worker waiting for new builds....
Fetching artifacts for build build_kobo(56535309): {u'size': 48693753, u'filename': u'artifacts.zip'}
Running command: curl...  (OK)
Invalid build artifact, failed to extract metadata from zipfile.

zip content:

  Length      Date    Time    Name
---------  ---------- -----   ----
 24069031  2018-03-09 07:14   koreader/koreader-kobo-arm-kobo-linux-gnueabihf-v2015.11-1552-gfdb8dfd_2018-03-09.targz
 24301519  2018-03-09 07:14   koreader/koreader-kobo-arm-kobo-linux-gnueabihf-v2015.11-1552-gfdb8dfd_2018-03-09.zip
@Frenzie

This comment has been minimized.

Member

Frenzie commented Mar 9, 2018

Indeed, it looks like it specifically wants the full regex without any additions. I'll fix it this afternoon or tonight.

Frenzie added a commit to Frenzie/koreader-misc that referenced this pull request Mar 9, 2018

Frenzie added a commit to koreader/koreader-misc that referenced this pull request Mar 9, 2018

Frenzie added a commit that referenced this pull request Mar 26, 2018

[fix] kodev update $VERSION for `run android`
As in 1606137, leftover from #3723.

Also replaced a few non-standard `which` by `command -v` as per new shellcheck rule.
See https://github.com/koalaman/shellcheck/wiki/SC2230

Frenzie added a commit that referenced this pull request Mar 26, 2018

[fix] kodev update $VERSION for `run android` (#3801)
As in 1606137, leftover from #3723.

Also replaced a few non-standard `which` by `command -v` as per new shellcheck rule.
See https://github.com/koalaman/shellcheck/wiki/SC2230
@poire-z

This comment has been minimized.

Contributor

poire-z commented Mar 27, 2018

I think there is another issue with Android with this git-rev change.
I hadn't updated koreader on android for some time, but did this morning.
Now, everytime you launch koreader, you get the spin circle for a few seconds.
Previously, it happened only once after we updated with a new apk: it notices the version changes, and the files were unzipped from the .7z that is inside the apk.
So, now, I guess that at each launch, the 7z files are extracted to the android filesystem.

https://github.com/koreader/android-luajit-launcher/blob/52a76c19e565ba42f0e5b438fd507f772886cfab/assets/install.lua#L15-L35

> rev="v2015.11-1604-ge21fe55_2018-03-26" -- from "git-rev"
> print( rev:match(".*%-(.*)") )
26

The 7z file is just named koreader-ge21fe55.7z

@Frenzie

This comment has been minimized.

Member

Frenzie commented Mar 27, 2018

Oh, now that you mention it I had vaguely noticed that but on my reasonably speedy desktop in the Android emulator that process takes less than a second.

It seems to me that there isn't much point to using REVISION instead of VERSION. It's only used for Android.

koreader/Makefile

Lines 9 to 11 in e21fe55

VERSION:=$(shell git describe HEAD)
VERSION:=$(VERSION)_$(shell git describe HEAD | xargs git show -s --format=format:"%cd" --date=short)
REVISION=$(shell git rev-parse --short HEAD)

Then we could just say local package_name = git-rev..".7z" instead of doing parsing stuff that might break.

@poire-z

This comment has been minimized.

Contributor

poire-z commented Mar 27, 2018

Seems fine to me

Frenzie added a commit to koreader/android-luajit-launcher that referenced this pull request Mar 29, 2018

[fix] assets/install.lua: be less precise about revision format
Leftover from koreader/koreader#3723

There's no need to match a specific pattern.

Frenzie added a commit to koreader/android-luajit-launcher that referenced this pull request Mar 29, 2018

[fix] assets/install.lua: be less precise about revision format (#82)
Leftover from koreader/koreader#3723

There's no need to match a specific pattern.

Frenzie added a commit to Frenzie/koreader that referenced this pull request Mar 29, 2018

[Android] Makefile: only use VERSION, not REVISION
Fixes always reinstalling resource package.

See koreader#3723 (comment)

Frenzie added a commit that referenced this pull request Mar 29, 2018

[Android] Makefile: only use VERSION, not REVISION (#3811)
Fixes always reinstalling resource package.

See #3723 (comment)

@poire-z poire-z referenced this pull request Apr 7, 2018

Closed

Slow start on boox i86HD #3843

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment