Switch from PRBool to bool (in comm-central)

RESOLVED FIXED in Thunderbird 10.0

Status

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: mwu, Assigned: mwu)

Tracking

unspecified
Thunderbird 10.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 4 obsolete attachments)

If m-c switches to bool from PRBool, comm-central needs to switch at the same time to avoid massive burning.
Depends on: 681189
Attachment #557273 - Flags: review?(mbanner)
This is a simplified version of the commands I used to convert mozilla-central.

The actual patch is a bit less than 2mb.
Attachment #557275 - Flags: review?(mbanner)
These appear to be the only two things necessary so far. It builds suite successfully on Linux.
Also, the commands in #2 end up touching ldap, which appears to be its own thing. Not sure what special things are necessary to get things landed over there.
For ldap c-sdk we need to file a bug in Directory / LDAP C SDK, and attach a patch or script there and get richm to review.

Once we've done that I can get it landed and into the tree.
(In reply to Mark Banner (:standard8) from comment #5)
> For ldap c-sdk we need to file a bug in Directory / LDAP C SDK, and attach a
> patch or script there and get richm to review.
> 
> Once we've done that I can get it landed and into the tree.

So, as it turns out, I only touched stuff in ldap/xpcom, which is in comm-central after all. This should make things easier.
Comment on attachment 557275 [details]
2. Commands to generate a patch that s/PRBool/bool/, v3

Somehow I can't get this to work. Various lines are missing a '\' on the end, and the mac version of sed doesn't like -i -e, you have to do:

sed -i '' -e ....

which I believe is right.

However, even with fixing that lot up, sed doesn't actually seem to do anything, and although the files get touched, there is nothing in the resultant diff.
Attachment #557275 - Flags: review?(mbanner) → review-
Comment on attachment 557273 [details] [diff] [review]
1. Convert C++ code in IDLs to bool

-  PRBool  isExternalAttachment;  // Flag for determining if the attachment is external
+  bool    isExternalAttachment;  // Flag for determining if the attachment is external

Will we not also need to bump the uuid for these interfaces when the change happens, so that binary extensions have something to detect against?
(In reply to Mark Banner (:standard8) from comment #8)
> Comment on attachment 557273 [details] [diff] [review]
> 1. Convert C++ code in IDLs to bool
> 
> -  PRBool  isExternalAttachment;  // Flag for determining if the attachment
> is external
> +  bool    isExternalAttachment;  // Flag for determining if the attachment
> is external
> 
> Will we not also need to bump the uuid for these interfaces when the change
> happens, so that binary extensions have something to detect against?

Binary extensions must be recompiled for every release now AFAIK.
(In reply to Mark Banner (:standard8) from comment #7)
> Comment on attachment 557275 [details]
> 2. Commands to generate a patch that s/PRBool/bool/, v3
> 
> Somehow I can't get this to work. Various lines are missing a '\' on the
> end, and the mac version of sed doesn't like -i -e, you have to do:
> 
> sed -i '' -e ....
> 
> which I believe is right.
> 
> However, even with fixing that lot up, sed doesn't actually seem to do
> anything, and although the files get touched, there is nothing in the
> resultant diff.

Opps. I forgot to insert a few \ while formatting this to look pretty.

But this does work when run on a GNU system userspace and there isn't much need to port it to other unix systems. I'll attach a fixed version and a patch generated from the commands.
Attachment #558510 - Flags: review?(mbanner)
Attachment #557275 - Attachment is obsolete: true
Hm apparently there are a number of cases where PR_FALSE/TRUE is specified twice on a line, so the PR_TRUE/PR_FALSE sed should be run twice to cover that case.

For example:
-+        bool isJunk = PR_FALSE, isNotJunk = false;
++        bool isJunk = false, isNotJunk = false;
Attachment #560634 - Attachment description: Sed script to do s/PRBool/bool/ → Sed script to do s/PRBool/bool/ for OSX
This version is easier to use but requires the sed script I just attached.
Attachment #558510 - Attachment is obsolete: true
Attachment #558510 - Flags: review?(mbanner)
Attachment #560636 - Flags: review?(mbanner)
Forgot to add a g at the end.
Attachment #560637 - Attachment is obsolete: true
Attachment #560634 - Attachment is obsolete: true
Attachment #560638 - Attachment description: Sed script to do s/PRBool/bool/ for Windows/OSX, v2 → Sed script to do s/PRBool/bool/ for Windows/Linux, v2
Attachment #557273 - Flags: review?(mbanner) → review+
Comment on attachment 560636 [details]
2. Script to do s/PRBool/bool/, v5

This looks good. I've not been able to test all of it, but the results seem sane.

Thanks for doing these patches/scripts.
Attachment #560636 - Flags: review?(mbanner) → review+
http://hg.mozilla.org/comm-central/rev/1c8c69af4d1d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 10.0
Depends on: 730572
You need to log in before you can comment on or make changes to this bug.