[gwt-contrib] Re: a patch for legacy argument support
So the story goes:- Lex committed this patch nearly verbatim to [EMAIL PROTECTED] - Freeland and I committed a slightly different patch to releases/[EMAIL PROTECTED] So when we do our next merge from 1.6 -> trunk, we'll probably get some merge-hell. :) On Mon, Nov 17, 2008 at 12:29 PM, Scott Blum <[EMAIL PROTECTED]> wrote: > Will review ASAP. Thanks, Freeland. > > > On Mon, Nov 17, 2008 at 12:12 PM, Freeland Abbott < > [EMAIL PROTECTED]> wrote: > >> Yeah, I'd misunderstood the deleteOnExit() doc to imply recursive >> deletion, but they're hanging around. Also a small bug, I'd failed to shift >> my random number (so I got 5 repeated digits, and a random space of 36 vice >> 36^5). Both fixes attached... >> It turns out that a try/finally to do the cleanup interacts poorly with >> System.exit (which seems to really exit, right then, and so skip my finally >> block)... thus the code duplication in GWTCompiler.main(). >> >> >> >> On Mon, Nov 17, 2008 at 11:24 AM, John Tamplin <[EMAIL PROTECTED]> wrote: >> >>> On Mon, Nov 17, 2008 at 11:15 AM, Freeland Abbott < >>> [EMAIL PROTECTED]> wrote: >>> Well, if we're going to do that, let's put it into Utility, so we have a general-purpose mkdtemp equivalent... attached. And, in that case, do we want such "spontaneous" workDirs to be (best-effort) cleaned up at shutdown, via File.deleteOnExit()? I wouldn't delete a manually-specified workDir, even in GWTCompiler, but an implicit one might keep the accumulated trash down... >>> >>> Right, I forgot to mention the cleanup in the earlier email. I think an >>> automatically generated one should be cleaned up, but I don't think >>> deleteOnExit will do it if it has files in it, since delete on a directory >>> with files will fail. In this case, we don't need the on-exit hook, since >>> we know it has to come back trhough GwtCompiler.main anyway. >>> >>> Which is a big enough change I probably shouldn't rely on your prior LGTM. >>> >>> I'll look at it closer shortly, but we really need Scott to look at it >>> since he made the original changes. >>> >>> >>> -- >>> John A. Tamplin >>> Software Engineer (GWT), Google >>> >>> >>> >>> >> > --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: a patch for legacy argument support
Will review ASAP. Thanks, Freeland. On Mon, Nov 17, 2008 at 12:12 PM, Freeland Abbott < [EMAIL PROTECTED]> wrote: > Yeah, I'd misunderstood the deleteOnExit() doc to imply recursive deletion, > but they're hanging around. Also a small bug, I'd failed to shift my random > number (so I got 5 repeated digits, and a random space of 36 vice 36^5). > Both fixes attached... > It turns out that a try/finally to do the cleanup interacts poorly with > System.exit (which seems to really exit, right then, and so skip my finally > block)... thus the code duplication in GWTCompiler.main(). > > > > On Mon, Nov 17, 2008 at 11:24 AM, John Tamplin <[EMAIL PROTECTED]> wrote: > >> On Mon, Nov 17, 2008 at 11:15 AM, Freeland Abbott < >> [EMAIL PROTECTED]> wrote: >> >>> Well, if we're going to do that, let's put it into Utility, so we have a >>> general-purpose mkdtemp equivalent... attached. And, in that case, do we >>> want such "spontaneous" workDirs to be (best-effort) cleaned up at shutdown, >>> via File.deleteOnExit()? I wouldn't delete a manually-specified workDir, >>> even in GWTCompiler, but an implicit one might keep the accumulated trash >>> down... >>> >> >> Right, I forgot to mention the cleanup in the earlier email. I think an >> automatically generated one should be cleaned up, but I don't think >> deleteOnExit will do it if it has files in it, since delete on a directory >> with files will fail. In this case, we don't need the on-exit hook, since >> we know it has to come back trhough GwtCompiler.main anyway. >> >> >>> Which is a big enough change I probably shouldn't rely on your prior >>> LGTM. >>> >> >> I'll look at it closer shortly, but we really need Scott to look at it >> since he made the original changes. >> >> >> -- >> John A. Tamplin >> Software Engineer (GWT), Google >> >> >> >> > --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: a patch for legacy argument support
Yeah, I'd misunderstood the deleteOnExit() doc to imply recursive deletion, but they're hanging around. Also a small bug, I'd failed to shift my random number (so I got 5 repeated digits, and a random space of 36 vice 36^5). Both fixes attached... It turns out that a try/finally to do the cleanup interacts poorly with System.exit (which seems to really exit, right then, and so skip my finally block)... thus the code duplication in GWTCompiler.main(). On Mon, Nov 17, 2008 at 11:24 AM, John Tamplin <[EMAIL PROTECTED]> wrote: > On Mon, Nov 17, 2008 at 11:15 AM, Freeland Abbott < > [EMAIL PROTECTED]> wrote: > >> Well, if we're going to do that, let's put it into Utility, so we have a >> general-purpose mkdtemp equivalent... attached. And, in that case, do we >> want such "spontaneous" workDirs to be (best-effort) cleaned up at shutdown, >> via File.deleteOnExit()? I wouldn't delete a manually-specified workDir, >> even in GWTCompiler, but an implicit one might keep the accumulated trash >> down... >> > > Right, I forgot to mention the cleanup in the earlier email. I think an > automatically generated one should be cleaned up, but I don't think > deleteOnExit will do it if it has files in it, since delete on a directory > with files will fail. In this case, we don't need the on-exit hook, since > we know it has to come back trhough GwtCompiler.main anyway. > > >> Which is a big enough change I probably shouldn't rely on your prior LGTM. >> > > I'll look at it closer shortly, but we really need Scott to look at it > since he made the original changes. > > > -- > John A. Tamplin > Software Engineer (GWT), Google > > > > --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~--- flagupdates-1.6-r4033-3.patch Description: Binary data
[gwt-contrib] Re: a patch for legacy argument support
On Mon, Nov 17, 2008 at 11:15 AM, Freeland Abbott < [EMAIL PROTECTED]> wrote: > Well, if we're going to do that, let's put it into Utility, so we have a > general-purpose mkdtemp equivalent... attached. And, in that case, do we > want such "spontaneous" workDirs to be (best-effort) cleaned up at shutdown, > via File.deleteOnExit()? I wouldn't delete a manually-specified workDir, > even in GWTCompiler, but an implicit one might keep the accumulated trash > down... > Right, I forgot to mention the cleanup in the earlier email. I think an automatically generated one should be cleaned up, but I don't think deleteOnExit will do it if it has files in it, since delete on a directory with files will fail. In this case, we don't need the on-exit hook, since we know it has to come back trhough GwtCompiler.main anyway. > Which is a big enough change I probably shouldn't rely on your prior LGTM. > I'll look at it closer shortly, but we really need Scott to look at it since he made the original changes. -- John A. Tamplin Software Engineer (GWT), Google --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: a patch for legacy argument support
On Mon, Nov 17, 2008 at 1:15 AM, John Tamplin <[EMAIL PROTECTED]> wrote: > > Shouldn't Link.java:157 refer to Link? > Yes; fixed, and thanks. > > Won't this mean if you just run the precompile step it will automatically > create one if not specified? I think it should fail if you run the steps > separately without supplying -workDir, and you only get a default if you run > GwtCompiler. > As written, yes. I'm okay with either specification... change attached. > > I don't like the race condition in ensureWorkDir -- I know Java doesn't > support the equivalent of mkdtemp, but I think rather than creating a unique > file and trying to reuse that name for a directory I would rather just > atomically create a random directory and try another if it fails -- > basically implementing mkdtemp ourselves. > Well, if we're going to do that, let's put it into Utility, so we have a general-purpose mkdtemp equivalent... attached. And, in that case, do we want such "spontaneous" workDirs to be (best-effort) cleaned up at shutdown, via File.deleteOnExit()? I wouldn't delete a manually-specified workDir, even in GWTCompiler, but an implicit one might keep the accumulated trash down... Which is a big enough change I probably shouldn't rely on your prior LGTM. --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~--- flagupdates-1.6-r4033-2.patch Description: Binary data
[gwt-contrib] Re: a patch for legacy argument support
On Sun, Nov 16, 2008 at 10:15 PM, Freeland Abbott < [EMAIL PROTECTED]> wrote: > The attached patch does three things, two aimed around issues that arose > with the CLI argument reworks last week and a third that just seemed like a > good idea, although less important: > >1. The -workDir argument you pulled together is great in context of our >ant tests and the task inside them, but the same >concurrency-with-a-shared-directory issue has bitten a few other users. > Absent an explicit -workDir from the user, the attached patch will choose >one with a random component (i.e. workdir will be >/tmp/gwt-tmp/somethingrandom/ on Unix) in GWTCompiler or Precompile, and >balk and refuse to run in the other two phases. >2. The -extra argument is a breaking change for legacy users; the >attached restores old functionality if -extra is unsupplied. It also warns >that it's putting "auxilliary" stuff in the meant-to-be-deployable output >directory; hopefully we can retire that change after a release or so. >3. The not-strictly-for-compatibility change is that, in >EmbeddedTomcatServer, -Dcatalina.base can be set to something not yet >created, and we'll make it (just as we do if it's not set). > > This is against 1.6 r4033, but I'd like to propose merging it up to trunk > quickly, since it's addressing a breaking change. > Shouldn't Link.java:157 refer to Link? Won't this mean if you just run the precompile step it will automatically create one if not specified? I think it should fail if you run the steps separately without supplying -workDir, and you only get a default if you run GwtCompiler. I don't like the race condition in ensureWorkDir -- I know Java doesn't support the equivalent of mkdtemp, but I think rather than creating a unique file and trying to reuse that name for a directory I would rather just atomically create a random directory and try another if it fails -- basically implementing mkdtemp ourselves. Otherwise, LGTM. -- John A. Tamplin Software Engineer (GWT), Google --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
