627956 - Fold widget/src/build into widget/src/windows

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Zack Weinberg (:zwol)]

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

Comment 1

[Brian R. Bondy [:bbondy]]

Jim is this something you would like?

Comment 2

[Jim Mathies [:jimm]]

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.

Comment 3

[Zack Weinberg (:zwol)]

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.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah, we should do this.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Another thing we should do is move everything in 'widget/src' into 'widget'. Dunno if there's a bug for that.

Comment 6

[Jim Mathies [:jimm]]

Isn't there an issue with our current version of hg where moving files causes a loss of history?

Comment 7

[Zack Weinberg (:zwol)]

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

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(Actually we should work around Mackall by patching our hgweb install, but this is all discussion for elsewhere.)

Comment 10

[Brian R. Bondy [:bbondy]]

Attachment: 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.

Comment 11

[Brian R. Bondy [:bbondy]]

Attachment: 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.

Comment 12

[Brian R. Bondy [:bbondy]]

try run looks good:
https://tbpl.mozilla.org/?tree=Try&rev=c56aeade4b48

Comment 13

[Brian R. Bondy [:bbondy]]

Attachment: Patch v3. Fixed makefile

Comment 14

[Brian R. Bondy [:bbondy]]

Pushed to mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/d26fad81f51a

Comment 15

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/d26fad81f51a

Comment 16

[[e-mail]]

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