Closed Bug 627956 Opened 9 years ago Closed 8 years ago

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

Categories

(Core :: Widget: Win32, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: zwol, Assigned: bbondy)

References

Details

Attachments

(1 file, 2 obsolete files)

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.)
Jim is this something you would like?
Assignee: nobody → netzen
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.
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.
Another thing we should do is move everything in 'widget/src' into 'widget'. Dunno if there's a bug for that.
Isn't there an issue with our current version of hg where moving files causes a loss of history?
(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.)
Blocks: 679226
No longer blocks: 679226
Depends on: 679226
Attached patch Patch v1. (obsolete) — Splinter Review
- 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.
Attached patch Patch v2. (obsolete) — Splinter Review
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
Attachment #585323 - Attachment is obsolete: true
Attachment #585502 - Flags: review?(roc)
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
Closed: 8 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.