- flashrom-gerrit - mail.coreboot.org

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

18 Jun '17

Hello Stefan Tauner, david.hendricks(a)gmail.com, Philippe Mathieu-Daudé, Paul Menzel, build bot (Jenkins), Nico Huber, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/19206 to look at the new patch set (#5). 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, 582 insertions(+), 125 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/19206/5 -- To view, visit https://review.coreboot.org/19206 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newpatchset Gerrit-Change-Id: I64eef21982cac0a0a7419bcd2c8a936672ae9cb2 Gerrit-Change-Number: 19206 Gerrit-PatchSet: 5 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

1 0

Change in flashrom[staging]: ich_descriptors: Fix more odd +1s
by Philippe Mathieu-Daudé (Code Review) 18 Jun '17

18 Jun '17

Philippe Mathieu-Daudé has posted comments on this change. ( https://review.coreboot.org/19044 ) Change subject: ich_descriptors: Fix more odd +1s ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/19044 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Ifa5455c999e90ff9121aed29f542d71ac9ca2b1c Gerrit-Change-Number: 19044 Gerrit-PatchSet: 4 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.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-Comment-Date: Sun, 18 Jun 2017 23:52:51 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: ichspi: Drop `dev` parameter from init functions
by Philippe Mathieu-Daudé (Code Review) 18 Jun '17

18 Jun '17

Philippe Mathieu-Daudé has posted comments on this change. ( https://review.coreboot.org/19460 ) Change subject: ichspi: Drop `dev` parameter from init functions ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/19460 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I0d4e7cc731364e86eff214b9022b842a577f9ef4 Gerrit-Change-Number: 19460 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 18 Jun 2017 23:51:41 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: fixup! Kill doit()
by Nico Huber (Code Review) 18 Jun '17

18 Jun '17

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/20250 ) Change subject: fixup! Kill doit() ...................................................................... fixup! Kill doit() Fix building with CONFIG_INTERNAL=no because force_boardmismatch doesn't exist when internal is not enabled. Change-Id: Id9e715f09ef934bc36221b3e72c578ae96e0a3af Signed-off-by: Urja Rannikko <urjaman(a)gmail.com> Reviewed-on: https://review.coreboot.org/20250 Reviewed-by: Nico Huber <nico.h(a)gmx.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M cli_classic.c 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: Nico Huber: Looks good to me, approved build bot (Jenkins): Verified diff --git a/cli_classic.c b/cli_classic.c index 0a09cfd..4c3db4d 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -553,7 +553,9 @@ flashrom_layout_set(fill_flash, layout); flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE, !!force); +#if CONFIG_INTERNAL == 1 flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE_BOARDMISMATCH, !!force_boardmismatch); +#endif flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_AFTER_WRITE, !dont_verify_it); flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_WHOLE_CHIP, !dont_verify_all); -- To view, visit https://review.coreboot.org/20250 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: merged Gerrit-Change-Id: Id9e715f09ef934bc36221b3e72c578ae96e0a3af Gerrit-Change-Number: 20250 Gerrit-PatchSet: 3 Gerrit-Owner: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: fixup! Kill doit()
by build bot (Jenkins) (Code Review) 18 Jun '17

18 Jun '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/20250 ) Change subject: fixup! Kill doit() ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/390/ : SUCCESS -- To view, visit https://review.coreboot.org/20250 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Id9e715f09ef934bc36221b3e72c578ae96e0a3af Gerrit-Change-Number: 20250 Gerrit-PatchSet: 2 Gerrit-Owner: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 18 Jun 2017 10:07:52 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: fixup! Kill doit()
by Urja Rannikko (Code Review) 18 Jun '17

18 Jun '17

Hello build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/20250 to look at the new patch set (#2). Change subject: fixup! Kill doit() ...................................................................... fixup! Kill doit() Fix building with CONFIG_INTERNAL=no because force_boardmismatch doesn't exist when internal is not enabled. Change-Id: Id9e715f09ef934bc36221b3e72c578ae96e0a3af Signed-off-by: Urja Rannikko <urjaman(a)gmail.com> --- M cli_classic.c 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/50/20250/2 -- To view, visit https://review.coreboot.org/20250 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id9e715f09ef934bc36221b3e72c578ae96e0a3af Gerrit-Change-Number: 20250 Gerrit-PatchSet: 2 Gerrit-Owner: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: Convert flashrom to git
by Nico Huber (Code Review) 17 Jun '17

17 Jun '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/19206 ) Change subject: Convert flashrom to git ...................................................................... Patch Set 4: (10 comments) Looks pretty good overall, except for the complex getrevision.sh changes. Splitting it out into a follow-up would get the other changes in (with a minor update, see inline comments), I suppose. https://review.coreboot.org/#/c/19206/4/Makefile File Makefile: https://review.coreboot.org/#/c/19206/4/Makefile@a1374 PS4, Line 1374: Why didn't I bikeshed this for being totally unrelated? https://review.coreboot.org/#/c/19206/4/Makefile@1396 PS4, Line 1396: @tar -cz --format=ustar -f $(EXPORTDIR)/flashrom-$(RELEASENAME).tar.gz -C $(EXPORTDIR)/ \ : $(TAROPTIONS) flashrom-$(RELEASENAME)/ : # Delete the exported directory again because it is most likely what's expected by the user. : @rm -rf $(EXPORTDIR)/flashrom-$(RELEASENAME) Missing quotes around everything with $(RELEASENAME). https://review.coreboot.org/#/c/19206/1/util/getrevision.sh File util/getrevision.sh: https://review.coreboot.org/#/c/19206/1/util/getrevision.sh@66 PS1, Line 66: "to the commit history from an upstream repository. If no git remote"\ > Done Looks good, but did you test it? Using echo this way there shouldn't be any line breaks in the output at all. https://review.coreboot.org/#/c/19206/1/util/getrevision.sh@69 PS1, Line 69: > not needed? Why not? The message should draw the user's attention and isn't usual program output (i.e. make progress). https://review.coreboot.org/#/c/19206/1/util/getrevision.sh@224 PS1, Line 224: urn > I'm wondering if this is all too elaborate. I'd rather keep it simple and s Sounds good, I was all for splitting from the beginning ;) We could just update getrevision.sh to use git for a start. Add the upcache + complex version string (we should start a discussion on the ML probably) later. https://review.coreboot.org/#/c/19206/1/util/getrevision.sh@224 PS1, Line 224: return : fi : done : } : : revision() { : local sha=$(git_last_commit "$1" 2>/dev/null) : # > What we'd need here instead is What I meant here is roughly: In the `-n "$up_remote"` case, we only construct $up_refs based on registered remotes and $upstream_branches. If the user just registered the upstream remote without ever fetching it, $up_refs will contain refs unknown to git. What we need to do is kind of force_update_upcache but for the detected already present remotes. We could spare us all that hassle and some of the already written code if we'd always use an upcache. https://review.coreboot.org/#/c/19206/1/util/getrevision.sh@257 PS1, Line 257: update_upcache || { echo "offline" ; return ; } : up_refs=$upcache_refs : fi : : # Find nearest commit contained in this branch that is also in any of the up_refs, i.e. the branch point : # of the current branch. This might be the latest commit if it's in any of the upstream branches. : loca > TBH, I'm not sure what to do about that here :-/ Don't worry. It's only a cosmetic issue. And I don't remember it accurately. https://review.coreboot.org/#/c/19206/1/util/getrevision.sh@269 PS1, Line 269: > Yep, now it is! Then we don't need the `if`. https://review.coreboot.org/#/c/19206/4/util/getrevision.sh File util/getrevision.sh: https://review.coreboot.org/#/c/19206/4/util/getrevision.sh@72 PS4, Line 72: "$upstream_url"\ Maybe indent it a little. https://review.coreboot.org/#/c/19206/1/util/git-hooks/pre-push File util/git-hooks/pre-push: https://review.coreboot.org/#/c/19206/1/util/git-hooks/pre-push@53 PS1, Line 53: "$commit" ]; then > If we're going that route, "Neither Signoff nor Ack were found..." would so Sounds better but is not what I meant: Both have to be present. -- To view, visit https://review.coreboot.org/19206 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I64eef21982cac0a0a7419bcd2c8a936672ae9cb2 Gerrit-Change-Number: 19206 Gerrit-PatchSet: 4 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-Comment-Date: Sat, 17 Jun 2017 13:31:43 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: fixup of "Kill doit()"
by Nico Huber (Code Review) 17 Jun '17

17 Jun '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/20250 ) Change subject: fixup of "Kill doit()" ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/#/c/20250/1//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/20250/1//COMMIT_MSG@7 PS1, Line 7: fixup of "Kill doit()" fixup! Kill doit() https://review.coreboot.org/#/c/20250/1//COMMIT_MSG@10 PS1, Line 10: doesnt doesn't -- To view, visit https://review.coreboot.org/20250 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Id9e715f09ef934bc36221b3e72c578ae96e0a3af Gerrit-Change-Number: 20250 Gerrit-PatchSet: 1 Gerrit-Owner: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 17 Jun 2017 12:06:37 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: fixup of "Kill doit()"
by Nico Huber (Code Review) 17 Jun '17

17 Jun '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/20250 ) Change subject: fixup of "Kill doit()" ...................................................................... Patch Set 1: Code-Review+2 > Re: the guards, then you'd need to build internal.o always (since > Makefile doesnt build it if no CONFIG_INTERNAL), and include an > useless piece of .bss in the binary. You dislike guards that much? I wouldn't mind the wasted .bss space when it makes the code more readable. I wrongly assumed we'd always compile internal.c (guess why, it has CONFIG_INTERNAL guards in it, OMG). Since it's not, your patch looks suddenly good :-P > Oh, the handling of that flag seems all kinds of weird to me, in > general. My fault. I needed to expose it through the library interface. Don't know why I didn't realize that the flashctx is the wrong place for the flags back then. Probably because there was no other context... > I could go with not having it in the flashctx, which would make it > an internal programmer parameter that flashrom_image_write happens > to look at (but there's guards there for it anyways already). > (Having it only in flashctx comes with the issue that the flashctx > doesnt exist when initializing the programmer...) The mid-term idea is to either make flashctx always available or add a programmer context (cf. `struct flash_programmer` in libflashrom.h). > Or it could be split into two parameters, but ... ehh, i'm not even > sure if having the flashctx part makes sense without internal, etc. Eh, no. It should just move (together with all other global variables) into a programmer context. > > This is very confusing to me and I'm here to fix the build, not fix > the architechture :P Agreed. -- To view, visit https://review.coreboot.org/20250 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Id9e715f09ef934bc36221b3e72c578ae96e0a3af Gerrit-Change-Number: 20250 Gerrit-PatchSet: 1 Gerrit-Owner: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 17 Jun 2017 11:57:59 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: fixup of "Kill doit()"
by Urja Rannikko (Code Review) 17 Jun '17

17 Jun '17

Urja Rannikko has posted comments on this change. ( https://review.coreboot.org/20250 ) Change subject: fixup of "Kill doit()" ...................................................................... Patch Set 1: Oh, the handling of that flag seems all kinds of weird to me, in general. I could go with not having it in the flashctx, which would make it an internal programmer parameter that flashrom_image_write happens to look at (but there's guards there for it anyways already). (Having it only in flashctx comes with the issue that the flashctx doesnt exist when initializing the programmer...) Or it could be split into two parameters, but ... ehh, i'm not even sure if having the flashctx part makes sense without internal, etc. This is very confusing to me and I'm here to fix the build, not fix the architechture :P -- To view, visit https://review.coreboot.org/20250 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Id9e715f09ef934bc36221b3e72c578ae96e0a3af Gerrit-Change-Number: 20250 Gerrit-PatchSet: 1 Gerrit-Owner: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 17 Jun 2017 11:09:04 +0000 Gerrit-HasComments: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit