Exclude allow/deny move in profile from git blame #4390
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fe0f975 (move whitelist/blacklist to allow/deny) is just a huge rename w/o effects to the profile. Ignoreing it in git blame to see which commits actually added/changed a allow/nodeny command. Configure git to use .git-blame-ignore-revs: git config blame.ignoreRevsFile .git-blame-ignore-revs
|
There are also some more commits which maybe make sense to add like my broken merge commit or the etc split commit. |
reinerh
approved these changes
Jul 7, 2021
|
nice! I like this. |
|
Cool! |
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jul 18, 2021
This reverts commit fe0f975. Note: This only reverts the changes from etc. The 4 aliases introduced on commit 45f2ba5 are mere, well, aliases. That is, they fail to address the different usability problems discussed on [netblue30#3447][3447] and in fact only make things more confusing (as has already been mentioned on [this][4379] and later comments). The main reason is that the aliases do not meaningfully map to the original commands. For example, the commands from each pair below seem like they would do the exact same thing: * `allow` and `nodeny` * `deny` and `noallow` Additionally, if these aliases are not the final commands, but only a test/work-in-progress, then keeping the wide-scale search/replace changes made on commit fe0f975 would only serve to cause confusion, as users of firejail-git, contributors and downstream projects might start changing the commands used on their profiles, only to later have to change them again, potentially to completely different commands. The sooner this is undone the better, as (besides the above reasons) the more profile changes there are between the original commit and the revert, the harder it is to e.g.: `git diff` versions of files across the following revision ranges: before the commit, after the commit but before the revert and after the revert. Note: This is still the case even if a commit is [ignored by `git blame`][4390]. So let us revert fe0f975 and only reapply similar large-scale changes once we have discussed and settled on better commands. How the revert was applied: Despite using the auto-generated message from `git revert`, to ensure correctness and to avoid conflicts the changes were reverted in different steps: Firstly, revert the files which can be safely reverted directly ("filestorevert"): # Find out which files have been changed on fe0f975, but have not # been changed afterwards and list them on "filestorevert" git show --pretty='' --name-only fe0f975 -- etc | LC_ALL=C sort >allfiles git diff --name-only fe0f975..master -- etc | LC_ALL=C sort >filestoignore comm -2 -3 allfiles filestoignore >filestorevert # Note: There are 3 extra files on filestoignore because they were # added after commit fe0f975 wc -l allfiles filestoignore filestorevert | head -n 3 # 797 allfiles # 8 filestoignore # 792 filestorevert # Automatically revert files in "filestorevert" # See https://stackoverflow.com/a/23401018/10095231 tr '\n' '\000' <filestorevert | xargs -0 git show fe0f975 -- | git apply --reverse printf 'Total files reverted:\n' git diff --name-only | wc -l # 792 Secondly, do some search/replace on the rest: tr '\n' '\000' <filestoignore | xargs -0 sed -i.bak \ -e 's/allow /whitelist /' -e 's/noallow /nowhitelist /' \ -e 's/deny /blacklist /' -e 's/nodeny /noblacklist /' \ -e 's/deny-nolog /blacklist-nolog /' find etc -name '*.bak' -print0 | xargs -0 rm Thirdly, verify the result. The following command shows the difference between all the changes in etc from before fe0f975 and this commit (inclusive): git diff fe0f975~1 -- etc From the output, it looks like all alias changes are fully reverted and that the other changes to etc (from after fe0f975) remain, so the revert seems to be done correctly. [3447]: netblue30#3447 [4379]: netblue30#4379 (comment) [4390]: netblue30#4390
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Aug 27, 2021
Add commit f43382f ("Revert "move whitelist/blacklist to allow/deny"") from PR netblue30#4410. As mentioned on commit b023b9a ("Exclude allow/deny move in profile from git blame") / PR netblue30#4390, commit fe0f975 ("move whitelist/blacklist to allow/deny") "is just a huge rename", and so is the revert of it. Note that there is a follow-up to f43382f: commit 2e4d52e ("Revert allow/deny additional files") (sort of related to netblue30#4421). It renames a bit too much, which is later fixed by commit 3836131 ("Fix zim and rednotebook"). Since these are small changes and since they involve regressions, neither commit is added.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fe0f975 (move whitelist/blacklist to
allow/deny) is just a huge rename¹ w/o effects to the profile. Ignoreing
it in git blame to see which commits actually added/changed a
allow/nodeny command.
Configure git to use .git-blame-ignore-revs:
EDIT: ¹ but it also fe0f975#diff-7c733ed074575806d2755a48dd858c22c318d0190737f64299fa6dda414ba29b and fe0f975#diff-9f33b84e2dc73f7861f9d367b48fe0e3cf23832184311457b6ebef69714c358c