Created attachment 31536 [details] Proposed patch on tomcat7 trunk When using the Manager to deploy a webapp remotely (uploading the war with an http PUT), the deployment status is incorrectly reported. It does not occur when deploying a WAR that is on the server filesystem. The proposed patch factors the check for the context state.
Comment on attachment 31536 [details] Proposed patch on tomcat7 trunk The first and second chunks of your patch change behavior (by adding an isAvailable check). Is that the real fix for this bug? In general, I'm not a fan of having a "check" method dump anything to the output writer. Would it be possible to re-write the patch with the re-factored method but not dump any output? Or, maybe just change the name so it's clear that stuff will be printed to the output writer (even though it should be obvious to the reader than the writer is bring used since it's passed as a parameter).
Created attachment 31554 [details] Proposed patch on tomcat7 trunk (In reply to Christopher Schultz from comment #1) > The first and second chunks of your patch change behavior (by adding an > isAvailable check). Is that the real fix for this bug? Yes, exactly. > > In general, I'm not a fan of having a "check" method dump anything to the > output writer. Would it be possible to re-write the patch with the > re-factored method but not dump any output? Or, maybe just change the name > so it's clear that stuff will be printed to the output writer (even though > it should be obvious to the reader than the writer is bring used since it's > passed as a parameter). OK, I renamed the method to "checkAndOutputContextDeploymentStatus", see new patch attached. If this is OK, I can commit it since I should still have commit rights. But I did not follow much tomcat dev lately : should I commit on 8.x then 7.x ? should I update the changelog in the same commit or different one ? Or if you prefer to commit this yourself, be my guest ;-)
(In reply to Sylvain Laurent from comment #2) > OK, I renamed the method to "checkAndOutputContextDeploymentStatus", see new > patch attached. I'm all for clear method names but that seems a little long outputDeploymentResult() should do. > If this is OK, I can commit it since I should still have commit rights. > But I did not follow much tomcat dev lately : should I commit on 8.x then > 7.x ? Yes. Commit to 8.0.x and then svn merge back to 7.0.x. > Should I update the changelog in the same commit or different one ? Ideally the same one. > Or if you prefer to commit this yourself, be my guest ;-) As I am looking at this now I'll do it (but you will get the credit/blame in the changelog).
Thanks. Applied to 8.0.x for 8.0.6 and 7.0.x for 7.0.54.