check kernel version and issue warning if necessary #180

shrx · 2015-06-23T12:37:15Z

Fixes #178
for real this time

popcornmix · 2015-06-23T12:40:47Z

The problem is the NOTICE.md mechanism is a general purpose scheme for warning about any change that may cause issues - not just for the 4.0 kernel.

The correct solution is for WARNING.md to include a git hash for the first update that requires the warning. E.g. a line like `HASH: abcdef0123456789abcdef``
Call this hash W.
The last rpi-update hash in in /boot/.firmware_revision. Call this U.
The latest hash you are updating to is L.

Then, if U < W && L >= W you display the warning.

shrx · 2015-06-23T12:48:53Z

How to achieve this when hashes are not incremental?

popcornmix · 2015-06-23T13:18:08Z

This repo is committed to linearly (without rebases/merges) so the date of the hashes should be in order.
If you run with JUST_CHECK=1 it prints a list of commits since you last updated. Grepping that for the HASH listed in NOTICE.md should say if you are including the WARNING commit.

now with smart hash comparison

shrx · 2015-06-24T07:17:07Z

I have updated the script so it can compare commit hashes. Also added HASH to NOTICE.md in rpi-firmware repo.

popcornmix · 2015-06-24T15:07:21Z

Looks promising.
Can you handle the case where HASH is not present in NOTICE.md (I think you should get the warning unconditionally)
Can you handle the case where /boot/.firmware_revision is not present (treat LOCAL_HASH date as very old).

Ruffio · 2016-04-30T06:40:25Z

@shrx any chance you would update this to @popcornmix spec so it can be merged?

Ruffio · 2016-05-18T10:06:41Z

@shrx it would be cool if you had the time to make the small changes so this PR could be merged.

shrx · 2016-05-18T15:05:01Z

Sorry, I completely forgot about this.
Will have a look at what needs to be done.

Handles the case where HASH is not present in NOTICE.md Handles the case where /boot/.firmware_revision is not present

shrx · 2016-05-19T07:15:58Z

@Ruffio @popcornmix I think everything is in order now, it should handle the case where HASH is not present in NOTICE.md and where /boot/.firmware_revision is not present.

Improved efficiency logic

popcornmix · 2016-05-19T13:51:00Z

Currently we have a dependency on curl but not wget, and this PR adds a dependency on wget.
Is is possible to switch to using curl for get_hash_date?

popcornmix · 2016-05-19T13:59:30Z

rpi-update

-		return
+	local NOTICE_HASH_HEAD=$(echo "$FULL_NOTICE" | head -1)
+	if [ $(echo "${NOTICE_HASH_HEAD}" | awk -F: '{print $1}') == "HASH" ]; then
+		local NOTICE_HASH_EXISTS = true


I get local: =': not a valid identifier`. The spaces need removing.

bash doesn't like spaces

switch to curl

shrx · 2016-05-19T16:10:08Z

@popcornmix can you check now? I removed the offending spaces (hopefully all) and switched to curl.

popcornmix · 2016-05-19T16:20:48Z

Should the curl include -L?
I think we needed to add that to all other uses as github sometimes redirects responses.

shrx · 2016-05-19T17:22:08Z

It can't hurt, I guess.

curl follows redirects

Ruffio · 2016-05-20T06:23:04Z

@popcornmix is this good to go?
I guess that the "add -L to curl in all other places" should be performed in another PR?

popcornmix · 2016-05-25T16:30:02Z

With the current NOTICE.md file, we discard the first line of the message.
I suspect we want to use NOTICE when NOTICE_HASH_EXISTS=true and FULL_NOTICE when NOTICE_HASH_EXISTS=false.

Don't discard lines if HASH is missing

Ruffio · 2016-06-29T20:12:39Z

@popcornmix merge?

lurch · 2016-06-30T09:19:52Z

rpi-update

+	if ${NOTICE_HASH_EXISTS}; then
+		local NEW_HASH=${FW_REV}
+		local LOCAL_lt_NOTICE=$(compare_hashes ${LOCAL_HASH} lt ${NOTICE_HASH})
+		local NEW_ge_NOTICE=$(compare_hashes ${NEW_HASH} ge ${NOTICE_HASH})


By calling get_hash_date inside compare_hashes, this results in four separate requests to curl.
If you rearranged the logic slightly, and got the hash dates before comparing them, then you'd only need three requests to curl.
I guess there's also no need to call get_hash_date in the case when LOCAL_HASH has been set to 0.
And is it possible that any of LOCAL_HASH, NOTICE_HASH or NEW_HASH could have the same value? That might allow you to (conditionally) eliminate another call to curl.

Ruffio · 2016-07-26T06:50:04Z

@shrx any chance you would do the last changes so we can get this PR merged? It would be awsome to get it in place. Think about it

changed `compare_hashes` logic to reduce the number of `curl` calls

Ruffio · 2016-07-27T12:38:03Z

@lurch @popcornmix ready to merge?

Ruffio · 2016-08-01T13:34:37Z

@popcornmix ready to merge?

Ruffio · 2016-08-17T20:33:57Z

@popcornmix ready to merge?

lurch · 2016-09-09T14:09:11Z

rpi-update

+			local LOCAL_HASH_DATE=$(get_hash_date ${LOCAL_HASH})
+		fi
+		local LOCAL_lt_NOTICE=$(compare_hashes "${LOCAL_HASH_DATE}" lt "${NOTICE_HASH_DATE}")
+		local NEW_ge_NOTICE=$(compare_hashes "${NEW_HASH_DATE}" ge "${NNOTICE_HASH_DATE}")


Just spotted that this line has ${NNOTICE_HASH_DATE} (note the two Ns) which I guess is a typo.

Ruffio · 2017-01-13T12:10:12Z

@shrx any chance you would update this PR?

shrx · 2017-01-24T11:05:07Z

@Ruffio what needs updating?

Ruffio · 2017-01-24T11:19:12Z

@shrx Have you read @lurch comments and observations? It also looks like there is a conflict to be resolved.

lurch · 2019-06-08T14:19:26Z

rpi-update

 	if [ -z "$NOTICE" ]; then
 		return
 	fi
+	local NOTICE_HASH_HEAD=$(echo "$FULL_NOTICE" | head -1)
+	if [ $(echo "${NOTICE_HASH_HEAD}" | awk -F: '{print $1}') == "HASH" ]; then
+		local NOTICE_HASH_EXISTS=true


For style-consistency with the rest of the code in this script, you should probably use 1 and 0 instead of true and false, and then change all your conditionals to e.g. if [[ ${NOTICE_HASH_EXISTS} -eq 1 ]]; then

lurch · 2019-06-08T14:31:45Z

rpi-update

@@ -281,6 +310,19 @@ function noobs_fix {

 }

+function get_hash_date {
+	commit_api="https://api.github.com/repos/Hexxeh/rpi-firmware/git/commits/"


I think for style-consistency this should be upper-cased to COMMIT_API, and it should be dynamically calculated from $REPO_URI in the same way that REPO_API is on line 364; in case the user supplies a custom REPO_URI.

In fact, perhaps it makes sense to have REPO_API=${REPO_URI/github.com/api.github.com\/repos} up at the top of the script, and then line 315 could use $REPO_API/git/commits/$1 and line 365 etc. could use $REPO_API/git/refs/heads/${BRANCH}

I've made this change in #279 🙂

Ruffio · 2020-12-25T09:31:33Z

@shrx is this still relevant?

check kernel version and issue warning if necessary

61b193c

Fixes Hexxeh#178

shrx mentioned this pull request Jun 23, 2015

check kernel version and issue warning if necessary #179

Closed

check kernel version and issue warning if necessary

c478341

now with smart hash comparison

Update rpi-update

47f760c

Handles the case where HASH is not present in NOTICE.md Handles the case where /boot/.firmware_revision is not present

Update rpi-update

18dea0d

Improved efficiency logic

popcornmix reviewed May 19, 2016
View reviewed changes

shrx added 2 commits May 19, 2016 18:04

Update rpi-update

4e5fe1f

bash doesn't like spaces

Update rpi-update

986365d

switch to curl

Update rpi-update

6d43989

curl follows redirects

Update rpi-update

9856b77

Don't discard lines if HASH is missing

lurch reviewed Jun 30, 2016
View reviewed changes

Update rpi-update

21b75dd

changed `compare_hashes` logic to reduce the number of `curl` calls

Update rpi-update

f5b32bf

lurch reviewed Sep 9, 2016
View reviewed changes

lurch reviewed Jun 8, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

check kernel version and issue warning if necessary #180

check kernel version and issue warning if necessary #180

shrx commented Jun 23, 2015

popcornmix commented Jun 23, 2015

shrx commented Jun 23, 2015

popcornmix commented Jun 23, 2015

shrx commented Jun 24, 2015

popcornmix commented Jun 24, 2015

Ruffio commented Apr 30, 2016 •

edited

Loading

Ruffio commented May 18, 2016

shrx commented May 18, 2016

shrx commented May 19, 2016

popcornmix commented May 19, 2016

popcornmix May 19, 2016

shrx commented May 19, 2016

popcornmix commented May 19, 2016

shrx commented May 19, 2016

Ruffio commented May 20, 2016

popcornmix commented May 25, 2016

Ruffio commented Jun 29, 2016

lurch Jun 30, 2016

Ruffio commented Jul 26, 2016

Ruffio commented Jul 27, 2016

Ruffio commented Aug 1, 2016

Ruffio commented Aug 17, 2016

lurch Sep 9, 2016

Ruffio commented Jan 13, 2017

shrx commented Jan 24, 2017

Ruffio commented Jan 24, 2017

lurch Jun 8, 2019

lurch Jun 8, 2019

lurch Jun 8, 2019

Ruffio commented Dec 25, 2020

check kernel version and issue warning if necessary #180

Are you sure you want to change the base?

check kernel version and issue warning if necessary #180

Conversation

shrx commented Jun 23, 2015

popcornmix commented Jun 23, 2015

shrx commented Jun 23, 2015

popcornmix commented Jun 23, 2015

shrx commented Jun 24, 2015

popcornmix commented Jun 24, 2015

Ruffio commented Apr 30, 2016 • edited Loading

Ruffio commented May 18, 2016

shrx commented May 18, 2016

shrx commented May 19, 2016

popcornmix commented May 19, 2016

popcornmix May 19, 2016

Choose a reason for hiding this comment

shrx commented May 19, 2016

popcornmix commented May 19, 2016

shrx commented May 19, 2016

Ruffio commented May 20, 2016

popcornmix commented May 25, 2016

Ruffio commented Jun 29, 2016

lurch Jun 30, 2016

Choose a reason for hiding this comment

Ruffio commented Jul 26, 2016

Ruffio commented Jul 27, 2016

Ruffio commented Aug 1, 2016

Ruffio commented Aug 17, 2016

lurch Sep 9, 2016

Choose a reason for hiding this comment

Ruffio commented Jan 13, 2017

shrx commented Jan 24, 2017

Ruffio commented Jan 24, 2017

lurch Jun 8, 2019

Choose a reason for hiding this comment

lurch Jun 8, 2019

Choose a reason for hiding this comment

lurch Jun 8, 2019

Choose a reason for hiding this comment

Ruffio commented Dec 25, 2020

Ruffio commented Apr 30, 2016 •

edited

Loading