Skip to content
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

Latest Version broke talismanrc with multiple files #314

Closed
fpaiva-tw opened this issue Aug 10, 2021 · 10 comments
Closed

Latest Version broke talismanrc with multiple files #314

fpaiva-tw opened this issue Aug 10, 2021 · 10 comments
Labels
bug priority On priority for picking up

Comments

@fpaiva-tw
Copy link

Describe the bug

Using the latest version if the commit has additions in more than 1 file, the filename pattern will only match the last pattern in the list "fileignoreconfig" of talismanrc.

To Reproduce

echo 'password: fake_password1' >> talisman_check_1.yml
echo 'password: fake_password2' >> talisman_check_2.yml
git add talisman_check_*
echo "fileignoreconfig:" > .talismanrc    
echo "- filename: talisman_check_1.yml">> .talismanrc
echo "  checksum: 2c6264526f072937cf754ed37e0a6e96346c20e6bb3274cc6074fc229161704c">> .talismanrc
echo "- filename: talisman_check_2.yml">> .talismanrc
echo "  checksum: 61c5affa3bb323e275213b1d940d1c85200c85202cd41ebef3e419b22da09cb5">> .talismanrc
git commit -m"test"

Expected behavior
The report should come clean, since both files are ignored

Screenshots

Screen Shot 2021-08-10 at 19 18 09

Desktop

  • OS: macOS Big Sur 11.5.1
@svishwanath-tw
Copy link
Collaborator

@fpaiva-tw thanks for reporting this. Am reverting the latest release.

@chenrui333
Copy link

@svishwanath-tw would it be better to create a new release roll forward? Thanks!

@svishwanath-tw
Copy link
Collaborator

@chenrui333 : That would generally be a good idea. The default installation/update script for talisman will revert to the last released version on github. However, with Homebrew in the picture, things do get complicated. I wonder if something could be worked out, the only other way I can think of is to tag and older release with a newer semver, that would be confusing too.

@svishwanath-tw
Copy link
Collaborator

@fpaiva-tw:

fileignoreconfig:
- filename: talisman_check1.yml
  checksum: f153120787768c753f444a33bbf5f6be77b2e8a274dc83e2c53010bd18d3e5e6
- filename: talisman_check2.yml
  checksum: 78297fc54bb676ff650b79d3255de471e664a3075caa89545a8c4a7f106f89d9

This is the talismanrc suggested by talisman. How did you get the checksums for talisman_check1.yml and talisman_check2.yml ?

@fpaiva-tw
Copy link
Author

@svishwanath-tw:

In my system the suggested talismanrc is:

fileignoreconfig:
- filename: talisman_check_1.yml
  checksum: 2c6264526f072937cf754ed37e0a6e96346c20e6bb3274cc6074fc229161704c
- filename: talisman_check_2.yml
  checksum: 61c5affa3bb323e275213b1d940d1c85200c85202cd41ebef3e419b22da09cb5

I used the checksum generated by the report. You can check the issue's attached screenshot for reference.
I also used the talisman --checksum=talisman_check_1.yml and talisman --checksum=talisman_check_2.yml and got the same results.

I am not sure why the checksum is different in my system then yours, but the bug is no concerning the checksum itself. Even if you use the checksum that your system is generating you can probably reproduce it.

@chenrui333
Copy link

I wonder if something could be worked out, the only other way I can think of is to tag and older release with a newer semver, that would be confusing too.

Agree, a plain re-tag of an old release is kind of weird. But from a technical perspective, we should not call the already rolled out a release back to pre-release again, that is also confusing.

It just means the release process needs to be more thoughtful and prevent the bad UX in the future.

Either way, I think a new release would be better. :)

@chenrui333
Copy link

I actually just found a pretty similar issue that happened to graphql-java 16.0 release. It was a problematic release, and the team create a emergency patch release
image

I think the situation is pretty similar like in here.

@svishwanath-tw
Copy link
Collaborator

@fpaiva-tw : Released a new version that fixes the issue, please download and verify.

@svishwanath-tw
Copy link
Collaborator

@chenrui333, @fpaiva-tw : Issues fixed and latest release is 1.23.0

@svishwanath-tw
Copy link
Collaborator

Closing issue as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug priority On priority for picking up
Projects
None yet
Development

No branches or pull requests

3 participants