Fold widget/src/build into widget/src/windows

RESOLVED FIXED in mozilla12

Status

()

Core
Widget: Win32
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: zwol, Assigned: bbondy)

Tracking

unspecified
mozilla12
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

7 years ago
The contents of widget/src/build belong in widget/src/windows.  Whoever does this, please also rename nsWinWidgetFactory.cpp to nsWidgetFactory.cpp for consistency with the other widget impls.

(I would do this myself but build-testing Windows is presently a pain, and it's post-FF4 material and I probably won't remember by the time it comes around again.)
(Assignee)

Comment 1

6 years ago
Jim is this something you would like?
(Assignee)

Updated

6 years ago
Assignee: nobody → netzen

Comment 2

6 years ago
Not sure if nsWinWidgetFactory.cpp is currently in use by other platforms, we should investigate. Also, we may be able to blow away the the rc file and the cursor files in res.. I'm not sure what those are for. Worth investigating, but not a high priority. cc'ing Roc as he might know more. These resources look ancient.
(Reporter)

Comment 3

6 years ago
widget/src/Makefile.in adds 'build' to DIRS if and only if it adds 'windows' to DIRS.  So unless something reaches into widget/src from elsewhere in the tree, I'm confident the directories can be merged.  I would recommend Just Doing It and worrying about possibly-obsolete files in a separate bug.
Yeah, we should do this.
Another thing we should do is move everything in 'widget/src' into 'widget'. Dunno if there's a bug for that.

Comment 6

6 years ago
Isn't there an issue with our current version of hg where moving files causes a loss of history?
(Reporter)

Comment 7

6 years ago
(In reply to Jim Mathies [:jimm] from comment #6)
> Isn't there an issue with our current version of hg where moving files
> causes a loss of history?

Yes, see http://mercurial.selenic.com/bts/issue1576 . However, my understanding is that this is not a permanent loss of information, only a failure of information-displaying commands (notably 'annotate') to understand the rename record.  Therefore, I do not think we should delay source tree reorganization because of it.
Zack's mostly right; no actual history is lost, but some Mercurial UI doesn't handle file moves well. "annotate" is actually fine, but "log" cuts off (but provides a link to the previous location). Matt Mackall thinks this is desirable behavior and wontfixed the bug :-(. The current state is not too bad and as good as it's going to get, so we may as well go ahead with file moves.
(Actually we should work around Mackall by patching our hgweb install, but this is all discussion for elsewhere.)
(Assignee)

Updated

6 years ago
Blocks: 679226
(Assignee)

Updated

6 years ago
No longer blocks: 679226
Depends on: 679226
(Assignee)

Comment 10

6 years ago
Created attachment 585028 [details] [diff] [review]
Patch v1.

- Removed unused source files and resources
- Renamed nsWinWidgetFactory.cpp to nsWidgetFactory.cpp
- Moved widget/build/nsWidgetFactory.cpp to widget/windows

Will request a review once the try results come in.
(Assignee)

Comment 11

6 years ago
Created attachment 585323 [details] [diff] [review]
Patch v2.

Regarding the .rc file and resource files that were removed, I found that the build failed because toolkit\library\xulrunner.rc(1) tries to #include "widget.rc".

I'm not sure if that's important or not but I re-added the files in the new location instead of removing.  If we want to remove them let's do that in a different bug.

Re-pushed to try, will request a review once it succeeds.
Attachment #585028 - Attachment is obsolete: true
(Assignee)

Comment 12

6 years ago
try run looks good:
https://tbpl.mozilla.org/?tree=Try&rev=c56aeade4b48
(Assignee)

Comment 13

6 years ago
Created attachment 585502 [details] [diff] [review]
Patch v3. Fixed makefile
Attachment #585323 - Attachment is obsolete: true
Attachment #585502 - Flags: review?(roc)
Attachment #585502 - Flags: review?(roc) → review+
(Assignee)

Comment 14

6 years ago
Pushed to mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/d26fad81f51a
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/d26fad81f51a
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Comment on attachment 585502 [details] [diff] [review]
Patch v3. Fixed makefile

> ifneq (,$(filter os2 cocoa qt android gonk,$(MOZ_WIDGET_TOOLKIT)))
> DIRS += $(MOZ_WIDGET_TOOLKIT)
> endif
> 
> ifeq ($(MOZ_WIDGET_TOOLKIT),windows)
>-DIRS += windows build
>+DIRS += windows
> endif
Could have written this:
ifneq (,$(filter windows os2 cocoa qt android gonk,$(MOZ_WIDGET_TOOLKIT)))
DIRS += $(MOZ_WIDGET_TOOLKIT)
endif
You need to log in before you can comment on or make changes to this bug.