681188 - Switch from PRBool to bool (in comm-central)

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Michael Wu [:mwu]]

If m-c switches to bool from PRBool, comm-central needs to switch at the same time to avoid massive burning.

Comment 1

[Michael Wu [:mwu]]

Attachment: 1. Convert C++ code in IDLs to bool

Comment 2

[Michael Wu [:mwu]]

Attachment: 2. Commands to generate a patch that s/PRBool/bool/, v3

This is a simplified version of the commands I used to convert mozilla-central.

The actual patch is a bit less than 2mb.

Comment 3

[Michael Wu [:mwu]]

These appear to be the only two things necessary so far. It builds suite successfully on Linux.

Comment 4

[Michael Wu [:mwu]]

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.

Comment 5

[Mark Banner (:standard8)]

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.

Comment 6

[Michael Wu [:mwu]]

(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 7

[Mark Banner (:standard8)]

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.

Comment 8

[Mark Banner (:standard8)]

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?

Comment 9

[Michael Wu [:mwu]]

(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.

Comment 10

[Michael Wu [:mwu]]

(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.

Comment 11

[Michael Wu [:mwu]]

Attachment: 2. Commands to generate a patch that s/PRBool/bool/, v4

Comment 12

[Michael Wu [:mwu]]

Attachment: Patch generated from the commands

Comment 13

[Michael Wu [:mwu]]

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;

Comment 14

[Michael Wu [:mwu]]

Attachment: Sed script to do s/PRBool/bool/ for OSX

Comment 15

[Michael Wu [:mwu]]

Attachment: 2. Script to do s/PRBool/bool/, v5

This version is easier to use but requires the sed script I just attached.

Comment 16

[Michael Wu [:mwu]]

Attachment: Sed script to do s/PRBool/bool/ for Windows/OSX

Comment 17

[Michael Wu [:mwu]]

Attachment: Sed script to do s/PRBool/bool/ for Windows/Linux, v2

Forgot to add a g at the end.

Comment 18

[Michael Wu [:mwu]]

Attachment: Sed script to do s/PRBool/bool/ for OSX, v2

Comment 19

[Mark Banner (:standard8)]

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.

Comment 20

[Michael Wu [:mwu]]

http://hg.mozilla.org/comm-central/rev/1c8c69af4d1d