Skip to content

gitian: fixed SC2001 regex #17680

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 9, 2019
Merged

gitian: fixed SC2001 regex #17680

merged 1 commit into from
Dec 9, 2019

Conversation

willyko
Copy link
Contributor

@willyko willyko commented Dec 5, 2019 •
edited by laanwj
Loading

Currently the gitian-win-signer.yml produces OUTFILE names without -unsigned stripped out
This is due to regex having an% in front of it

$ INFILE="bitcoin-0.19.0-win64-setup-unsigned.exe"
$ echo "${INFILE/%-unsigned}"
bitcoin-0.19.0-win64-setup-unsigned.exe
$ echo "${INFILE/-unsigned}"
bitcoin-0.19.0-win64-setup.exe

Fixes #17361

@willyko
gitian: fixed SC2001 regex
c966ff1 
the `-` is not a special symbol and should not have `%` in front of it.
@maflcko
Copy link
Member

maflcko commented Dec 5, 2019

Why is SC2001 disabled?

@maflcko maflcko requested review from dongcarl and hebasto December 5, 2019 21:22
@dongcarl
Copy link
Contributor

dongcarl commented Dec 5, 2019 •
edited
Loading

Why is SC2001 disabled?

It isn't, but it seems that it didn't pick up our mistake and assumed % was part of the pattern

@hebasto We need to fix the rest of them too... 17f81e9#diff-43ab305e977a53cc028bfb0eaaacf989

@willyko
Copy link
Contributor Author

willyko commented Dec 5, 2019

Why is SC2001 disabled?

It isn't, but it seems that it didn't pick up our mistake and assumed % was part of the pattern

@hebasto We need to fix the rest of them too... 17f81e9#diff-43ab305e977a53cc028bfb0eaaacf989

The rest are ok. . is a special character so it requires that % in front of it

@laanwj
Copy link
Member

laanwj commented Dec 6, 2019

Currently the gitian-win-signer.yml produces OUTFILE names without -unsigned stripped out

When was this problem introduced? On releases, this renaming definitely works and a filename without -unsigned is produced.

@willyko
Copy link
Contributor Author

willyko commented Dec 6, 2019

Currently the gitian-win-signer.yml produces OUTFILE names without -unsigned stripped out

When was this problem introduced? On releases, this renaming definitely works and a filename without -unsigned is produced.

#17361 8 days ago. There's gitian builds in PR but did not include codesigned gitian builds

@laanwj
Copy link
Member

laanwj commented Dec 6, 2019

@willyko ok, good to know this doesn't affect any releases
thanks for checking (and fixing)

ACK c966ff1

@hebasto
Copy link
Member

hebasto commented Dec 6, 2019

#17361 8 days ago. There's gitian builds in PR but did not include codesigned gitian builds

Sorry for introducing a bug.

@willyko Thank you for fixing!

From bash docs:

${parameter/pattern/string}

The pattern is expanded to produce a pattern just as in filename expansion. Parameter is expanded and the longest match of pattern against its value is replaced with string. The match is performed according to the rules described below (see Pattern Matching). If pattern begins with ‘/’, all matches of pattern are replaced with string... If pattern begins with ‘%’, it must match at the end of the expanded value of parameter. If string is null, matches of pattern are deleted and the / following pattern may be omitted.

So this

$ INFILE="bitcoin-0.19.0-win64-setup-unsigned.exe"
$ echo "${INFILE/%-unsigned}"
bitcoin-0.19.0-win64-setup-unsigned.exe

does not work because -unsigned does not match at the end of "bitcoin-0.19.0-win64-setup-unsigned.exe".

The rest are ok. . is a special character so it requires that % in front of it

I don't think . is a special character that requires % in front of it. The purpose of % is described in docs (see above).

hebasto
hebasto approved these changes Dec 6, 2019
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c966ff1

laanwj added a commit that referenced this pull request Dec 9, 2019
@laanwj
Merge #17680: gitian: fixed SC2001 regex
ac09180 
c966ff1 gitian: fixed SC2001 regex (willyk)

Pull request description:

  Currently the gitian-win-signer.yml produces OUTFILE names without `-unsigned` stripped out
  This is due to regex having an`%` in front of it
  ```
  $ INFILE="bitcoin-0.19.0-win64-setup-unsigned.exe"
  $ echo "${INFILE/%-unsigned}"
  bitcoin-0.19.0-win64-setup-unsigned.exe
  $ echo "${INFILE/-unsigned}"
  bitcoin-0.19.0-win64-setup.exe
  ```

  Fixes #17361

ACKs for top commit:
  laanwj:
    ACK c966ff1
  hebasto:
    ACK c966ff1

Tree-SHA512: 954547f9dfa4cab4def5f284d4837c21f0e6fed7454a04e83e6b1b7d3fd3f9661ea657047f0d8162f6591909d32ef2f72e801b2f3a44cbb1131ac344cb913a69
@laanwj laanwj merged commit c966ff1 into bitcoin:master Dec 9, 2019
@MarkLTZ MarkLTZ mentioned this pull request Apr 4, 2020
Closed
MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Apr 10, 2020
@MarkLTZ
script: Lint Gitian descriptors with ShellCheck bitcoin#17361
c864dba 
gitian: fixed SC2001 regex bitcoin#17680
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 17, 2021
@laanwj @PastaPastaPasta
Merge bitcoin#17680: gitian: fixed SC2001 regex
f12ca55 
c966ff1 gitian: fixed SC2001 regex (willyk)

Pull request description:

  Currently the gitian-win-signer.yml produces OUTFILE names without `-unsigned` stripped out
  This is due to regex having an`%` in front of it
  ```
  $ INFILE="bitcoin-0.19.0-win64-setup-unsigned.exe"
  $ echo "${INFILE/%-unsigned}"
  bitcoin-0.19.0-win64-setup-unsigned.exe
  $ echo "${INFILE/-unsigned}"
  bitcoin-0.19.0-win64-setup.exe
  ```

  Fixes bitcoin#17361

ACKs for top commit:
  laanwj:
    ACK c966ff1
  hebasto:
    ACK c966ff1

Tree-SHA512: 954547f9dfa4cab4def5f284d4837c21f0e6fed7454a04e83e6b1b7d3fd3f9661ea657047f0d8162f6591909d32ef2f72e801b2f3a44cbb1131ac344cb913a69
thelazier pushed a commit to thelazier/dash that referenced this pull request Sep 25, 2021
@laanwj @thelazier
Merge bitcoin#17680: gitian: fixed SC2001 regex
b95eab4 
c966ff1 gitian: fixed SC2001 regex (willyk)

Pull request description:

  Currently the gitian-win-signer.yml produces OUTFILE names without `-unsigned` stripped out
  This is due to regex having an`%` in front of it
  ```
  $ INFILE="bitcoin-0.19.0-win64-setup-unsigned.exe"
  $ echo "${INFILE/%-unsigned}"
  bitcoin-0.19.0-win64-setup-unsigned.exe
  $ echo "${INFILE/-unsigned}"
  bitcoin-0.19.0-win64-setup.exe
  ```

  Fixes bitcoin#17361

ACKs for top commit:
  laanwj:
    ACK c966ff1
  hebasto:
    ACK c966ff1

Tree-SHA512: 954547f9dfa4cab4def5f284d4837c21f0e6fed7454a04e83e6b1b7d3fd3f9661ea657047f0d8162f6591909d32ef2f72e801b2f3a44cbb1131ac344cb913a69
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hebasto hebasto hebasto approved these changes

@dongcarl dongcarl Awaiting requested review from dongcarl

Assignees
No one assigned
Labels
Scripts and tools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@willyko @maflcko @dongcarl @laanwj @hebasto @fanquake