May 2017 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[staging]: Convert flashrom to git
by Anonymous Coward (Code Review) 15 May '17

15 May '17

david.hendricks(a)gmail.com has posted comments on this change. ( https://review.coreboot.org/19206 ) Change subject: Convert flashrom to git ...................................................................... Patch Set 1: (20 comments) Makefile, util/getrevision.sh, and util/git-hooks/pre-push still need some work and many of your PS1 comments still need to be addressed, but I tried to handle some of the low-hanging fruit. https://review.coreboot.org/#/c/19206/1//COMMIT_MSG Commit Message: PS1, Line 28: With this patch the version string includes the last reachable git tag, : number of commits since said tag in the upstream branches (if any), : the name of said upstream branch, number of commits since that branch : (if any), and the shortened git hash. This seems way over-complicated with no visible use case. It also makes the version confusing and difficult to parse programmatically. PS1, Line 63: 3 commits since last tag in upstream's staging branch and : 4 local commits on top of that, and some local uncommitted changes too Honestly, who cares? If somebody gets on the mailing list and presents this info, what value does it provide? AFAICT the only interesting part is the sha and "-dirty". I suppose one could make an argument for the branch name, but as you pointed out there's no guarantee that the branches are synchronized so even that info may just be misleading. PS1, Line 84: Acked-by: Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at> > I'd drop this since it's not this revision he acked. Done. While we're at it, should I add your Signed-off-by? By the time we're done you'll have contributed a significant chunk of the rework. https://review.coreboot.org/#/c/19206/1/util/getrevision.sh File util/getrevision.sh: PS1, Line 41: upcache_refs > uninitialized Done PS1, Line 44: # We need to update our upstream information sometimes so that we can create detailed version information. : # To that end the code below fetches parts of the upstream repository via https. : # This takes about one second or less under normal circumstances. > Do we break lines only at full stops now? This looks like a single Agreed, and done. Long lines can help readability in some cases, comments are not one of them. PS1, Line 50: forced_update > This is not about updating in general but updating the "upcache". Done PS1, Line 58: update > This is not about updating in general but updating the "upcache". Done Line 66: upstream repositories as git remote to rely on that information. > Is here supposed to start a new paragraph? Please separate them Done I avoided breaking commands to make copy+paste easier for the user and put the URL on its own line to try and avoid having it expand wider than the other lines. PS1, Line 69: >&2 not needed? PS1, Line 73: >&2 not needed? PS1, Line 77: >&2 not needed? PS1, Line 86: "$1" > This is an empty string if $1 is not set. It still works, but git I think the way this is used we'll always have non-empty $1, though it's not very intuitive since we need to trace the argument all the way back to main(). Maybe we should add something to abort and indicate a bug in the script if the "$1" is empty? Same for other functions, too. PS1, Line 218: previse > precise? Done, and reformatted as with the above comments. PS1, Line 224: -n "$up_remote" > Just that there is a upstream remote registered doesn't imply I'm wondering if this is all too elaborate. I'd rather keep it simple and stick with sha1 + dirty, and add upstream URL while we're at it. Those are easy to obtain and I don't think I've ever cared to know about "commits since the last tag in an unrelated upstream branch". All this extra stuff is interesting git-fu, but all it accomplishes is to make the version string confusing and impossible to parse programmatically. At the very least, I think we should split most of this hunk into a separate patch for now. LMK what you think. https://review.coreboot.org/#/c/19206/1/util/git-hooks/pre-push File util/git-hooks/pre-push: PS1, Line 1: #!/bin/sh > But it looks like a bash script to me. Since we probably want to avoid bashisms in general for git hooks, I got rid of the array by using a simple space-delimited string instead. PS1, Line 25: ( stable staging ) changed to "stable staging" Line 41: else > Why put an else here if we exited anyway? Good question. Fixed. PS1, Line 58: "${precious_branches[@]}" updated to iterate thru strings in $precious_branches Line 60: nonreachable=$(git rev-list $remote_sha ^$local_sha) > Maybe add `| head -1` the list could be long. Done PS1, Line 63: pusing > *pushing Done -- To view, visit https://review.coreboot.org/19206 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I64eef21982cac0a0a7419bcd2c8a936672ae9cb2 Gerrit-PatchSet: 1 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: david.hendricks(a)gmail.com Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: Convert flashrom to git
by David Hendricks (Code Review) 15 May '17

15 May '17

Hello build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/19206 to look at the new patch set (#2). Change subject: Convert flashrom to git ...................................................................... Convert flashrom to git - Drop support for Subversion in the getrevision script and Makefile. - Add .gitignore and .gitattributes file (the latter to limit exports). - Restore modification dates of the exported files from the SCM. - Stop exporting SCM log dumps to CHANGELOG. This makes no sense. - Remove djgpp-dos target (it is not different to other x-compilations). - Do not export the pre-"compiled" manpage. It can be generated like anything else from the code dump when we export the respective variable. The latter is added with this change. - Add some initial client-side git hooks * When committing check for obvious stuff you never want anyway: - white space errors * When pushing to the upstream repository check mandatory rules: - existing signoffs and acks in all new commits - no deletions or creation of branches - do not rewrite history of the precious branches, even if forced - Change version string of flashrom as follows. Previously, we included the last stable version according to a hard- coded string in the Makefile and appended the subversion revision number. With this patch the version string includes the last reachable git tag, number of commits since said tag in the upstream branches (if any), the name of said upstream branch, number of commits since that branch (if any), and the shortened git hash. In case there are uncommitted changes a "-dirty" indicator is also added. The case of unknown versions is explicitly handled in getrevision instead of simply appending "-unknown" to a hardcoded release number. The version information is either taken from an existing git remote pointing to an upstream repository (or a known mirror), or if that is not available - with the user's consent - a shadow copy is fetched from the upstream repo that is updated on every build (usually takes less than a second). In the following some examples of the version string changes are shown. Basically we print the distance to the last known upstream tag, which comprises any upstream commits since that tag as well as local commits on top of that. Additionally we handle upstream's stable and staging branches specially. Old output: flashrom v0.9.7-r1716 on Linux 3.8.0-6-generic (x86_64) New output variants: Build of the 0.9.99 release without any changes: flashrom v0.9.99-e4f6643 on Linux 3.13.0-76-generic (x86_64) 5 commits since last tag in upstream's stable branch: flashrom v0.9.99-5-stable-e4f6643-dirty on Linux 3.13.0-76-generic (x86_64) 3 commits since last tag in upstream's staging branch and 4 local commits on top of that: flashrom v0.9.99-3-staging-4-e4f6643 on Linux 3.13.0-76-generic (x86_64) 3 commits since last tag in upstream's staging branch and 4 local commits on top of that, and some local uncommitted changes too: flashrom v0.9.99-3-staging-4-e4f6643-dirty on Linux 3.13.0-76-generic (x86_64) 3 commits since the last tag in an unrelated upstream branch (e.g., a stable release *branch* such as 0.9.99.x) or local branch: flashrom v0.9.99-3-e4f6643 on Linux 3.13.0-76-generic (x86_64) No tags reachable from current commit (generic git fallback): flashrom d95935a version on Linux 3.13.0-76-generic (x86_64) Not in a repository: flashrom unknown version on Linux 3.13.0-76-generic (x86_64) NOTE: This patch is adapted from Stefan Tauner's original commit ( f5dd7ce1), but uses coreboot's commit-msg hook which includes support for generating and appending Change-Id which is now mandatory when pushing to the review server. Change-Id: I64eef21982cac0a0a7419bcd2c8a936672ae9cb2 Signed-off-by: Stefan Tauner <stefan.tauner(a)alumni.tuwien.ac.at> Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- A .gitattributes A .gitignore M Makefile M flashrom.c M util/getrevision.sh A util/git-hooks/applypatch-msg A util/git-hooks/commit-msg A util/git-hooks/install.sh A util/git-hooks/pre-applypatch A util/git-hooks/pre-commit A util/git-hooks/pre-push A util/git-hooks/wrapper.sh 12 files changed, 572 insertions(+), 124 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/19206/2 -- To view, visit https://review.coreboot.org/19206 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I64eef21982cac0a0a7419bcd2c8a936672ae9cb2 Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[stable]: Mark Spansion S25FL128P(0) tested for PREW
by Paul Menzel (Code Review) 13 May '17

13 May '17

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/19531 ) Change subject: Mark Spansion S25FL128P(0) tested for PREW ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/19531 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4b4e3673f7019f38a3b6ab2f7002bbe56dc6de07 Gerrit-PatchSet: 3 Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-Owner: Timothy Pearson <tpearson(a)raptorengineering.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Timothy Pearson <tpearson(a)raptorengineering.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in flashrom[stable]: Mark Spansion S25FL128P(0) tested for PREW
by Timothy Pearson (Code Review) 12 May '17

12 May '17

Timothy Pearson has removed a vote on this change. Change subject: Mark Spansion S25FL128P(0) tested for PREW ...................................................................... Removed Code-Review-2 by Timothy Pearson <tpearson(a)raptorengineering.com> -- To view, visit https://review.coreboot.org/19531 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: deleteVote Gerrit-Change-Id: I4b4e3673f7019f38a3b6ab2f7002bbe56dc6de07 Gerrit-PatchSet: 3 Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-Owner: Timothy Pearson <tpearson(a)raptorengineering.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Timothy Pearson <tpearson(a)raptorengineering.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[stable]: Mark Spansion S25FL128P(0) tested for PREW
by build bot (Jenkins) (Code Review) 12 May '17

12 May '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19531 ) Change subject: Mark Spansion S25FL128P(0) tested for PREW ...................................................................... Patch Set 3: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/304/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/310/ : SUCCESS -- To view, visit https://review.coreboot.org/19531 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4b4e3673f7019f38a3b6ab2f7002bbe56dc6de07 Gerrit-PatchSet: 3 Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-Owner: Timothy Pearson <tpearson(a)raptorengineering.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Timothy Pearson <tpearson(a)raptorengineering.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in flashrom[stable]: Mark Spansion S25FL128P(0) tested for PREW
by Timothy Pearson (Code Review) 12 May '17

12 May '17

Hello build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/19531 to look at the new patch set (#3). Change subject: Mark Spansion S25FL128P(0) tested for PREW ...................................................................... Mark Spansion S25FL128P(0) tested for PREW Read and write tested on ASUS KGPE-D16 with ASMB4 BMC module installed. Change-Id: I4b4e3673f7019f38a3b6ab2f7002bbe56dc6de07 Signed-off-by: Timothy Pearson <tpearson(a)raptorengineering.com> --- M flashchips.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/31/19531/3 -- To view, visit https://review.coreboot.org/19531 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4b4e3673f7019f38a3b6ab2f7002bbe56dc6de07 Gerrit-PatchSet: 3 Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-Owner: Timothy Pearson <tpearson(a)raptorengineering.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Timothy Pearson <tpearson(a)raptorengineering.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: Convert flashrom to git
by David Hendricks (Code Review) 12 May '17

12 May '17

David Hendricks has posted comments on this change. ( https://review.coreboot.org/19206 ) Change subject: Convert flashrom to git ...................................................................... Patch Set 1: (3 comments) A few drive-by comments, I still need to dive into the getrivision.sh and Makefile changes. https://review.coreboot.org/#/c/19206/1/util/git-hooks/install.sh File util/git-hooks/install.sh: PS1, Line 17: ../../ > This assumption looks rather odd given the `git rev-parse` above. Yep, that appears to do the trick! https://review.coreboot.org/#/c/19206/1/util/git-hooks/pre-push File util/git-hooks/pre-push: PS1, Line 53: No Signoff or Ack > This reads abiguous. I think it wants to say "Either Signoff or Ack not fou If we're going that route, "Neither Signoff nor Ack were found..." would sounds better IMO. (http://dictionary.cambridge.org/us/grammar/british-grammar/questions-and-ne…) PS1, Line 57: # Make _really_ sure we do not rewrite precious history : for lbranch in "${precious_branches[@]}"; do : if [ "$remote_ref" = "refs/heads/$lbranch" ]; then : nonreachable=$(git rev-list $remote_sha ^$local_sha) : if [ -n "$nonreachable" ]; then : echo "Only fast-forward pushes are allowed on $lbranch." >&2 : echo "$nonreachable is not included in $remote_sha while pusing to $remote_ref" >&2 : exit 1 : fi : fi : done > The code looks good, I was just worried that we reinvent the wheel. There are other examples of people using this on stackoverflow so I guess it's generally accepted (e.g http://stackoverflow.com/a/15760683) I guess we may as well just use receive.denyNonFastForwards until we come to a point where we want new branches? -- To view, visit https://review.coreboot.org/19206 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I64eef21982cac0a0a7419bcd2c8a936672ae9cb2 Gerrit-PatchSet: 1 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: dediprog: Add socket support for SF600
by build bot (Jenkins) (Code Review) 12 May '17

12 May '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19673 ) Change subject: dediprog: Add socket support for SF600 ...................................................................... Patch Set 3: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/303/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/309/ : SUCCESS -- To view, visit https://review.coreboot.org/19673 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5fd4133f08882d60ac596273ab8aa9dab893c9cd Gerrit-PatchSet: 3 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in flashrom[staging]: dediprog: Add socket support for SF600
by David Hendricks (Code Review) 12 May '17

12 May '17

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/19673 to look at the new patch set (#3). Change subject: dediprog: Add socket support for SF600 ...................................................................... dediprog: Add socket support for SF600 This enables use of the socket rather than clip on SF600 programmers. Change-Id: I5fd4133f08882d60ac596273ab8aa9dab893c9cd Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M dediprog.c 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/19673/3 -- To view, visit https://review.coreboot.org/19673 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5fd4133f08882d60ac596273ab8aa9dab893c9cd Gerrit-PatchSet: 3 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: dediprog: Add socket support for SF600
by build bot (Jenkins) (Code Review) 12 May '17

12 May '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19673 ) Change subject: dediprog: Add socket support for SF600 ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/302/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/308/ : SUCCESS -- To view, visit https://review.coreboot.org/19673 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5fd4133f08882d60ac596273ab8aa9dab893c9cd Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2017