Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 315890

Summary: Valgrind location cannot be overridden unless Valgrind is present in PATH
Product: [Tools] Linux Tools Reporter: Elliott Baron <ebaron>
Component: ValgrindAssignee: Elliott Baron <ebaron>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: overholt, rgrunber, sgehwolf
Version: 0.5.1   
Target Milestone: ---   
Hardware: PC   
OS: Linux   
Whiteboard:

Description Elliott Baron CLA 2010-06-05 16:55:22 EDT 
If a copy Valgrind is not present in the user's PATH, the code that checks the preference page for a manually set location is never executed. This defeats most of the purpose of having the preference.
Comment 1 Elliott Baron CLA 2010-06-06 19:19:49 EDT 
I have committed a fix that moves the preference checking ahead of calling the which command. The which command is now only used if the preference is not set manually. The preference checking itself is moved to the ValgrindCommand class.

Now what happens is when the plugin first needs to access the location of the Valgrind binary, it calls ValgrindCommand.whichValgrind(). This checks for the path set in a preference. If a path is not set, it calls the which command. The resulting path from either the preference store or the which command is cached for subsequent accesses. To force an update when the user changes the preference page, the ValgrindLaunchPlugin registers itself as a listener on that preference and invalidates the cached location when this preference is changed.

As Andrew mentioned for commits this close to Helios:
** All commits over the next 5 days must have an associated bug **
This bug is noted in the Changelogs.

** All commits over the next 5 days should be reviewed by at least one
   other committer **
Roland, would you be able to review the changes? You would be the ideal candidate since you implemented the preference page.

** All commits over the next 5 days should have a test case to verify
   that they do indeed fix what they intend to **
LocationPreferenceTest tests this bug specifically. It fails before the changes and succeeds after.
Comment 2 Roland Grunberg CLA 2010-06-07 10:07:14 EDT 
Reproduced bug, and have confirmed that it is fixed in trunk.