Bugzilla – Full Text Bug Listing
| Summary: | Listen to updates in dev.properties during runtime | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Kaloyan Raev <kaloyan> | ||||||||
| Component: | Framework | Assignee: | Thomas Watson <tjwatson> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P3 | CC: | dimitar.giormov, jeffmcaffer, l.kirchev, tjwatson | ||||||||
| Version: | 3.6.1 | Flags: | tjwatson:
review?
(jeffmcaffer) |
||||||||
| Target Milestone: | 3.7 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Linux | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Kaloyan Raev
Seems we could get by with simply re-reading the dev.properties when a bundle is resolved. This has to happen before a class loader can get created. Would probably want to do some simply time-stamp check to see if the file has changed before re-reading, but does not seem unreasonably hard to do. Besides that, we may need to rethink how we manage this dev-mode stuff. There are other issues with how we do it today. For example, the shape of the bundle is not 100% what it would be when you build and deploy it. Marking for 3.7. A contributed patch would make this more likely to happen ;-) Created attachment 185302 [details]
This patch adds reading of dev.properties each time a classloader is created, in case the file has changed
The patch makes a change DevClassPathHelper.
Previously dev.properties file was statically read upon initialization of this class. Subsequently, the class path was taken when a classloader gets created, but no check was made if the file had changed.
The patch adds reading of the dev.properties file (a check is made if it has changed) each time when a call to one of the methods for getting class path is made.
(In reply to comment #3) > Created an attachment (id=185302) [details] > This patch adds reading of dev.properties each time a classloader is created, > in case the file has changed > > The patch makes a change DevClassPathHelper. > > Previously dev.properties file was statically read upon initialization of this > class. Subsequently, the class path was taken when a classloader gets created, > but no check was made if the file had changed. > > The patch adds reading of the dev.properties file (a check is made if it has > changed) each time when a call to one of the methods for getting class path is > made. Thanks Lazar! I like the direction. I have the ollowing observations: - The patch currently pays attention to "osgi.dev" property changes. I prefer that we only read the osgi.dev property once. - Since the patch has to the re-read osgi.dev property each time, it must reconstruct the URL to the location each time. - The patch assumes the URL uses the file: protocol. I would prefer to stash the File devLocation of dev.properties in the static initializer if and only if the file: protocol was used for osgi.dev. Then based on the fact that the devLocation != null do the appropriate call to updateDevProperties(). The method updateDevProperties() would simply do the timestamp checking and reloading of the properties. Created attachment 185408 [details]
Update for the dev.properties patch
Tom, thanks for the remarks. Here I provide the revised patch.
Created attachment 185463 [details]
updated patch
Thanks, I modified the patch to fix an issue if a non file protocol was used. In this case the properties would never have been loaded. I also added synchronization when calling updateProperties and loading content from the cached properties.
Patch released. Thank you Lazar and Thomas! We've manually applied the patch to the 3.6.x code line and tested it successfully with one of our adopter's products. Thomas, is it possible to include this patch in the 3.6.2 release? This will enable us to get benefit of this change some months earlier - while we are still on Helios. As far as I can understand the patch is relatively straightforward and safe. (In reply to comment #8) > Thomas, is it possible to include this patch in the 3.6.2 release? This will > enable us to get benefit of this change some months earlier - while we are > still on Helios. As far as I can understand the patch is relatively > straightforward and safe. We don't typically add enhancements to point releases. The overall idea is pretty simple but the extra sync and reading required to implement this does have risk when considering for a point release. Jeff, thoughts? I would like you to review the code if we are considering this for 3.6.2. |