On 03/03/2015 15:48, Konstantin Kolinko wrote: > 2015-03-03 17:47 GMT+03:00 <ma...@apache.org>: >> Author: markt >> Date: Tue Mar 3 14:47:12 2015 >> New Revision: 1663715 >> >> URL: http://svn.apache.org/r1663715 >> Log: >> In directly fix https://bz.apache.org/bugzilla/show_bug.cgi?id=57251 > > You meant "Indirectly".
I did. > 1. The above ordering of setLastModified vs unpacking is wrong. > > Adding files to the directory while unpacking the war should change > its modification time. Your "setLastModified" call will be invalidated > by later changes. I'll check that and fix it. > 2. I wonder why I do not see any check for directory time stamp in > this commit. It only compares time stamps of xml and war files. ExpndWar should be correct. HostConfig needs fixing. > 3. Generally, I am not sure how well setting modification time of a > directory works cross-platform. There may be some edge cases here. I > am more used to file modification times. > > Probably modern OSes are OK, but there may be some issues with drives > accessed over network. (Just a guess. I do not have an actual > example.) > > This can be solved by allowing to opt-out from this feature - see 4. below. Absent an actual problem I'd rather not add another configuration option. I should be able to test if setting the timestamp works and skip the test if it doesn't. The only catch is that the code that checks the timestamp and the code that sets the timestamp are widely separated at the moment. A little refactoring may be required. > 4. If you are going to backport this, I intend to backport it to 8 and no further unless there is a demand for it from the users. > I think we need to allow to opt out of last modified check on the > expanded directory. > > Setting the date is OK - we are ignoring the boolean return value of > that method, checking the date has to be configurable. > > Use case: > The expanded directory is created via some other legacy method. > > E.g. by unpacking the war by some configuration tool. There may be > some legacy scripts doing such unpacking. > Maybe chef recipes, maybe RPMs. Personally I would just pack an > expanded directory into the RPM, but I do not know what people will be > doing. > > Someone can do unpacking from a script to save time on subsequent > Tomcat startup, > or if the same appbase is shared by several Tomcat instances. > > The script may be not smart enough to read timestamp of a file and set > timestamp on the directory. (As a future help: "touch -r reffile" > can be used to copy a timestamp from another file). > > > One more example: the war and expanded directory are stored in svn or > git. Those do not track time stamps of directories. Disabling unpackWARs should suffice (or is necessary for correct operation in some cases) for those use cases. The change needs to be clearly documented in the docs, migration guide and the release announcement. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
