The NSPR and NSS dependency system is unreliable. Whenever we update Mozilla to use a new snapshot of NSPR or NSS, we must remember to add or remove a whitespace line to the respective trick header file, in order to force a full rebuild. (as documented at https://developer.mozilla.org/en/Updating_NSPR_or_NSS_in_mozilla-central ) I've repeatedly forgotten to do that, therefore I want it automated. The proposal is to change the udpate_nss and update_nspr commands in client.py, and automatically add a blank line (or delete a blank line) at the end of the file. For NSPR the file is mozilla/nsprpub/config/prdepend.h For NSS the file is mozilla/security/coreconf/coreconf.dep

I wrote this code in a bash script: TAIL_EMPTY_COUNT=`tail -2 security/coreconf/coreconf.dep |grep -c "^$"` if [ $TAIL_EMPTY_COUNT == 2 ]; then # remove head --lines=-1 security/coreconf/coreconf.dep > security/coreconf/coreconf.dep.tmp mv -f security/coreconf/coreconf.dep.tmp security/coreconf/coreconf.dep else # append echo "" >> security/coreconf/coreconf.dep fi But client.py is python, and I found a way to translate the above bash into python: import subprocess depname = "security/coreconf/coreconf.dep" tmpname = depname + ".tmp" p1 = subprocess.Popen(["tail", "-2", depname], stdout=subprocess.PIPE) p2 = subprocess.Popen(["grep", "-c", "^$"], stdin=p1.stdout, stdout=subprocess.PIPE) p1.stdout.close() # Allow p1 to receive a SIGPIPE if p2 exits. output = p2.communicate()[0].strip() if output == "2": print "deleting" with open(tmpname, "w") as tmpfile: subprocess.call(["head", "--lines=-1", depname], stdout=tmpfile) tmpfile.close() subprocess.call(["mv", "-f", tmpname, depname]) else: print "appending" with open(depname, "a") as appendfile: appendfile.write("\n") appendfile.close() This works for me, I'll attach a patch that integrates this logic into client.py

Can you please review for mistakes? Thanks I've included toggle_nspr and toggle_nss commands, allowing you to easily test on your own. We can remove these commands if you don't like them.

FYI, head --lines=-1 filename will print the contents of "filename" except the trailing line.

Comment on attachment 651890 [details] [diff] [review] Patch v1 Please use python APIs instead of calling external processes (with options not supported on all platforms, on top of that (head --lines is a GNUism, it's not supported on OSX, at the very least))

Comment on attachment 652008 [details] [diff] [review] Patch v2 sigh. while trying to understand the surprising behaviour of my script, I discovered the output of "range" is one less than I had expected.

This appears to work fine.

Comment on attachment 652014 [details] [diff] [review] Patch v3 Review of attachment 652014 [details] [diff] [review]: ----------------------------------------------------------------- ::: client.py @@ +91,5 @@ > + Otherwise we'll add a blank line.""" > + f = open(depname, "r") > + lines = [] > + for line in f: > + lines.append(line.rstrip("\n")) lines = open(depname, "r").readlines() @@ +98,5 @@ > + if numlines < 1: > + print "unexpected short file" > + return > + > + if (lines[numlines-1].strip() == ''): if not lines[-1]: @@ +102,5 @@ > + if (lines[numlines-1].strip() == ''): > + # trailing line is blank, removing it > + with open(depname, "w") as outfile: > + for i in range(0, numlines-1): > + outfile.write("%s\n" % lines[i]); outfile.writelines(lines[:-1]) @@ +103,5 @@ > + # trailing line is blank, removing it > + with open(depname, "w") as outfile: > + for i in range(0, numlines-1): > + outfile.write("%s\n" % lines[i]); > + outfile.close() when using with, you don't close. And since that only leaves one line under with, you might as well do open(depname, "w").writelines(lines[:-1]) @@ +108,5 @@ > + else: > + # adding blank line > + with open(depname, "a") as appendfile: > + appendfile.write("\n") > + appendfile.close() likewise

(In reply to Mike Hommey [:glandium] from comment #8) > if not lines[-1]: lines[-1].strip(), even.

> > + if (lines[numlines-1].strip() == ''): > > if not lines[-1]: Is is really equivalent with the statement involving strip()? What if the line contains whitespace only?

> Is is really equivalent with the statement involving strip()? Seems it is, mysterious python!

(In reply to Kai Engert (:kaie) from comment #11) > > Is is really equivalent with the statement involving strip()? > > Seems it is, mysterious python! No, I had not made that change. Now I tested your proposal, and it's not equivalent. I suggest to use patch v4 (which doesn't use your proposal), because my approach will also remove a whitespace-only lines.

(In reply to Kai Engert (:kaie) from comment #13) > (In reply to Kai Engert (:kaie) from comment #11) > > > Is is really equivalent with the statement involving strip()? > > > > Seems it is, mysterious python! > > No, I had not made that change. Now I tested your proposal, and it's not > equivalent. > > I suggest to use patch v4 (which doesn't use your proposal), > because my approach will also remove a whitespace-only lines. See comment 9.

sorry, had missed that comment thanks for your review

Comment on attachment 652218 [details] [diff] [review] patch v5 Review of attachment 652218 [details] [diff] [review]: ----------------------------------------------------------------- ::: client.py @@ +86,5 @@ > cwd=os.path.join(topsrcdir, parent)) > print "CVS export end: " + datetime.datetime.utcnow().strftime("%Y-%m-%d %H:%M:%S UTC") > > +def toggle_trailing_blank_line(depname): > + """If the trailing line is re empty, then we'll delete it. "re empty" ? @@ +90,5 @@ > + """If the trailing line is re empty, then we'll delete it. > + Otherwise we'll add a blank line.""" > + lines = open(depname, "r").readlines() > + numlines = len(lines) > + if numlines < 1: if not lines @@ +91,5 @@ > + Otherwise we'll add a blank line.""" > + lines = open(depname, "r").readlines() > + numlines = len(lines) > + if numlines < 1: > + print "unexpected short file" print >>sys.stderr, "...

thanks Mike! final fixes applied, and unnecessary test commands removed.

Comment on attachment 652333 [details] [diff] [review] patch v6 Review of attachment 652333 [details] [diff] [review]: ----------------------------------------------------------------- ::: client.py @@ +91,5 @@ > + Otherwise we'll add a blank line.""" > + lines = open(depname, "r").readlines() > + if not lines: > + print >>sys.stderr, "unexpected short file" > + return Weird indentation

m2sger: instead of saying just "weird", it would be good if you rather clarified what you find weird about it, and what you propose instead