Issue 1363 - Make sure source code archives created by release automation consider the new dependen…

Issue 1363 - Make sure source code archives created by release automation consider the new dependen…

[Sebastian Noack, 2014-09-18 16:20:06]

http://codereview.adblockplus.org/5193970430771200/diff/5629499534213120/rele...
File releaseAutomation.py (right):

http://codereview.adblockplus.org/5193970430771200/diff/5629499534213120/rele...
releaseAutomation.py:24: with open(output, 'wb') as handle:
You can just pass the filename to tarfile.open() and save this line and a level
of indentation.

http://codereview.adblockplus.org/5193970430771200/diff/5629499534213120/rele...
releaseAutomation.py:30: with tarfile.open(fileobj=StringIO(data), mode='r:') as
repoarchive:
I would prefer to directly pass stdout of the subprocess to the tarfile.open(),
instead loading the output into memory just to wrap it into a StringIO:

hg = subprocess.check_output(['hg', 'archive', ...], stdout=subprocess.PIPE)
try:
  with tarfile.open(fileobj=hg.stdout, mode='r:') as repoarchive:
    ...
finally:
  hg.stdout.close()
  hg.wait()

[Sebastian Noack, 2014-09-18 16:21:23]

http://codereview.adblockplus.org/5193970430771200/diff/5629499534213120/rele...
File releaseAutomation.py (right):

http://codereview.adblockplus.org/5193970430771200/diff/5629499534213120/rele...
releaseAutomation.py:30: with tarfile.open(fileobj=StringIO(data), mode='r:') as
repoarchive:
On 2014/09/18 16:20:06, Sebastian Noack wrote:
> hg = subprocess.check_output(['hg', 'archive', ...], stdout=subprocess.PIPE)
hg = subprocess.Popen(['hg', 'archive', ...], stdout=subprocess.PIPE)

[Wladimir Palant, 2014-09-18 18:40:47]

http://codereview.adblockplus.org/5193970430771200/diff/5629499534213120/rele...
File releaseAutomation.py (right):

http://codereview.adblockplus.org/5193970430771200/diff/5629499534213120/rele...
releaseAutomation.py:24: with open(output, 'wb') as handle:
Done, don't remember why the original code did it this way.

http://codereview.adblockplus.org/5193970430771200/diff/5629499534213120/rele...
releaseAutomation.py:30: with tarfile.open(fileobj=StringIO(data), mode='r:') as
repoarchive:
Turns out that the ingredient I was missing here was mode='r|', without it
tarfile will try to seek the stream. Implemented without caching all the data
now.

I didn't bother waiting for the hg process however - what's the point if tarfile
will read until EOF anyway and an exception will terminate both processes?

[Sebastian Noack, 2014-09-18 18:58:12]

http://codereview.adblockplus.org/5193970430771200/diff/5629499534213120/rele...
File releaseAutomation.py (right):

http://codereview.adblockplus.org/5193970430771200/diff/5629499534213120/rele...
releaseAutomation.py:30: with tarfile.open(fileobj=StringIO(data), mode='r:') as
repoarchive:
On 2014/09/18 18:40:48, Wladimir Palant wrote:
> I didn't bother waiting for the hg process however - what's the point if
tarfile
> will read until EOF anyway and an exception will terminate both processes?

Killing zombies. ;)

Terminated processes stay around until their parent process terminates or
retrieved the exit code of the subprocess by calling wait.

So you should always wait for subprocesses, just like you always should close
file descriptors and release any other resource.

https://en.wikipedia.org/wiki/Zombie_process

[Wladimir Palant, 2014-09-18 19:23:57]

http://codereview.adblockplus.org/5193970430771200/diff/5629499534213120/rele...
File releaseAutomation.py (right):

http://codereview.adblockplus.org/5193970430771200/diff/5629499534213120/rele...
releaseAutomation.py:30: with tarfile.open(fileobj=StringIO(data), mode='r:') as
repoarchive:
On 2014/09/18 18:58:12, Sebastian Noack wrote:
> On 2014/09/18 18:40:48, Wladimir Palant wrote:
> > I didn't bother waiting for the hg process however - what's the point if
> tarfile
> > will read until EOF anyway and an exception will terminate both processes?
> 
> Killing zombies. ;)
> 
> Terminated processes stay around until their parent process terminates or
> retrieved the exit code of the subprocess by calling wait.
> 
> So you should always wait for subprocesses, just like you always should close
> file descriptors and release any other resource.
> 
> https://en.wikipedia.org/wiki/Zombie_process

Done.

[Sebastian Noack, 2014-09-18 20:43:50]

LGTM