Issue 292760043: i#1888: SELinux test needs to handle present but disabled
|
Created:
8 years, 1 month ago by algrant109 Modified:
8 years ago Reviewers:
bruening CC:
dynamorio-devs_googlegroups.com Visibility:
Public. |
DescriptionCommit log for first patchset:
---------------
i#1888: SELinux test needs to handle present but disabled
---------------
Patch Set 1 #Patch Set 2 : Committed #Patch Set 3 : Committed #Patch Set 4 : Committed #Patch Set 5 : Committed #Patch Set 6 : Committed #
MessagesTotal messages: 12
The code LGTM, but the commit message should have a Fixes line, right? Also, in general, consider adding a body to the commit message with a sentence or two of further explanation: as you can see in the history, almost none of our commit messages are just the single header line.
Sign in to reply to this message.
On 2016/03/04 15:37:34, bruening wrote: > The code LGTM, but the commit message should have a Fixes line, right? Also, in > general, consider adding a body to the commit message with a sentence or two of > further explanation: as you can see in the history, almost none of our commit > messages are just the single header line. The commit message template (unless you did not run the devsetup script?) has the format we expect: header line, blank line, body, and if it fixes the issue then a Fixes line.
Sign in to reply to this message.
I ran the devsetup script and used the git review script, but it was assuming it could pop up a browser on the local machine. upload.py has an option to suppress this but there seems to be no way to access this option from the review script. So I had to grab an access token and put it in my home directory and then run git review. Maybe I missed filling in a template? On Fri, Mar 4, 2016 at 3:42 PM, <bruening@google.com> wrote: > On 2016/03/04 15:37:34, bruening wrote: > >> The code LGTM, but the commit message should have a Fixes line, right? >> > Also, in > >> general, consider adding a body to the commit message with a sentence >> > or two of > >> further explanation: as you can see in the history, almost none of our >> > commit > >> messages are just the single header line. >> > > The commit message template (unless you did not run the devsetup > script?) has the format we expect: header line, blank line, body, and if > it fixes the issue then a Fixes line. > > https://codereview.appspot.com/292760043/ >
Sign in to reply to this message.
On 2016/03/04 17:22:32, algrant109 wrote: > I ran the devsetup script and used the git review script, but it was > assuming it could pop up a browser on the local machine. upload.py has an > option to suppress this but there seems to be no way to access this option > from the review script. So I had to grab an access token and put it in my > home directory and then run git review. Maybe I missed filling in a > template? The template is on the local commit to git and is unrelated to a review sent to the rietveld server. It is a script in .git/hooks/commit-msg. Does it fail to run when you commit?
Sign in to reply to this message.
Committed as https://github.com/DynamoRIO/dynamorio/commit/7e64263a940d745e5d1c0ce940eeb84... Final commit log: --------------- %B ---------------
Sign in to reply to this message.
Committed as https://github.com/DynamoRIO/dynamorio/commit/7e64263a940d745e5d1c0ce940eeb84... Final commit log: --------------- %B ---------------
Sign in to reply to this message.
On 2016/03/30 14:02:11, algrant109 wrote: > Committed as > https://github.com/DynamoRIO/dynamorio/commit/7e64263a940d745e5d1c0ce940eeb84... > > Final commit log: > --------------- > %B > --------------- ?
Sign in to reply to this message.
Committed as https://github.com/DynamoRIO/dynamorio/commit/7e64263a940d745e5d1c0ce940eeb84... Final commit log: --------------- i#1888: SELinux test needs to handle present but disabled The tests need special handling if SELinux is enabled. Previously this was checked for by looking for a tool called 'selinuxenabled', but this gives a false positive if SELinux is installed but disabled. So we go further and run the tool and look at its return code. Fixes #1888 Review-URL: https://codereview.appspot.com/292760043 ---------------
Sign in to reply to this message.
Committed as https://github.com/DynamoRIO/dynamorio/commit/7e64263a940d745e5d1c0ce940eeb84... Final commit log: --------------- i#1888: SELinux test needs to handle present but disabled The tests need special handling if SELinux is enabled. Previously this was checked for by looking for a tool called 'selinuxenabled', but this gives a false positive if SELinux is installed but disabled. So we go further and run the tool and look at its return code. Fixes #1888 Review-URL: https://codereview.appspot.com/292760043 ---------------
Sign in to reply to this message.
Committed as https://github.com/DynamoRIO/dynamorio/commit/7e64263a940d745e5d1c0ce940eeb84... Final commit log: --------------- i#1888: SELinux test needs to handle present but disabled The tests need special handling if SELinux is enabled. Previously this was checked for by looking for a tool called 'selinuxenabled', but this gives a false positive if SELinux is installed but disabled. So we go further and run the tool and look at its return code. Fixes #1888 Review-URL: https://codereview.appspot.com/292760043 ---------------
Sign in to reply to this message.
On Wed, Mar 30, 2016 at 3:09 PM, <bruening@google.com> wrote: > On 2016/03/30 14:02:11, algrant109 wrote: > >> Committed as >> > > > https://github.com/DynamoRIO/dynamorio/commit/7e64263a940d745e5d1c0ce940eeb84... > > Final commit log: >> --------------- >> %B >> --------------- >> > > ? > > https://codereview.appspot.com/292760043/ > My install of git was too old. I guess the script could check for this, I've raised #1916.
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
