Bug 25213

Summary: merge in windows patches in upstream dbus
Product: dbus Reporter: Ralf Habacker <ralf.habacker>
Component: coreAssignee: Thiago Macieira <thiago>
Status: RESOLVED FIXED QA Contact: John (J5) Palmieri <johnp>
Severity: normal    
Priority: medium CC: js, julien.isorce, ralf.habacker, tml
Version: unspecified   
Hardware: Other   
OS: Windows (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: initial patch from Tor Lillqvist

Description Ralf Habacker 2009-11-21 05:44:00 UTC
Created attachment 31367 [details] [review]
initial patch from Tor Lillqvist

This bug is an initial request for merging in the patches from Tor Lillqvist. He wrote: 


I spent some hours cherry-picking most of the commits from the
dbus4win-noncetcp branch of the dbus4win git repository to the master
branch of my local clone of the upstream dbus repository. (Manually
editing when necessary to make the commits apply.) It now builds on
Windows (using MinGW from MSYS, using autogen.sh and make), but I
obviously still need to make sure that it also works. Once I am
reasonably confident in it, what could/should be done to get it
upstream? (My local git repository is not accessible from the
outside.)
Comment 1 Ralf Habacker 2009-11-21 07:12:24 UTC
> Did you check the http://tml.pp.fi/dbus-dbus4win-merge.diff I pointed to in my
>mail? 

only the code parts 

> It is the output of git format-patch, so there are indeed separate easily
>digestable pieces. (114 of them...)

I see, I did not know about this nice feature before - grouping patches in on patch file 

>Do I understand correctly that you now then want the small pieces to be
>combined together again into a small number of larger patches? 

This depends on the features git have - I only have a very basic git-knowledge 

At least commit messages need to be consolidated, there are some windbus or dbus2win related internal messages, which for my opinion should not be included in the dbus commit message. See below for some examples 

------------------------------------------------------------
Subject: [PATCH 011/114] merge in all changes done in windbus.sf.net/trunk related to adopting windbus to dbus-1.2 API-changes;

r778 | chehrlic | 2008-04-23 20:11:35 +0200 (Wed, 23 Apr 2008) | 2 lines
update trunk to 1.2.1
this also means that we don't have a cvs version in trunk anymore - we have to work on a release version instead. Shouldn't be that big problem. If you need it,plz
open a branch.

r789 | chehrlic | 2008-08-13 12:32:03 +0200 (Wed, 13 Aug 2008) | 1 line
update to 1.2.3

r794 | chehrlic | 2008-10-23 19:35:39 +0200 (Thu, 23 Oct 2008) | 1 line
update to 1.2.4

r806 | chehrlic | 2009-01-18 00:18:46 +0100 (Sun, 18 Jan 2009) | 1 line
update to 1.2.12 - not tested yet!
-----------------------------------------------------------

I guess this should be shrinked to 

------------------------------
adopting dbus-1.2 API-changes 
------------------------------
 
or similar 

What's your approach ?  







Comment 2 Tor Lillqvist 2009-11-21 08:55:19 UTC
> At least commit messages need to be consolidated, 
> there are some windbus or
> dbus2win related internal messages,
> which for my opinion should not be included
> in the dbus commit message.

Yeah, unfortunately the git commit messages in the dbus4win repository are not always as nice as one might hope. Some of them have very long lines (a result of the git client they used, I guess). Whether they can/should be changed or not depends on how the merge is done. I am certainly no git expert, but I *think* that for such commits that can be simply cherry-picked (i.e. used as such letting git handle applying the commit from one repository into another), one can't edit the commit message, nor can one combine several such commits into one. Or is it possible?
Comment 3 Ralf Habacker 2009-11-23 10:19:14 UTC
(In reply to comment #2)

> I am certainly no git expert, 
me too 
> but I *think* that for such commits that can be simply cherry-picked (i.e. used as
> such letting git handle applying the commit from one repository into another),
> one can't edit the commit message, nor can one combine several such commits
> into one. Or is it possible?
> 

Using the following procedure will work without any deeper git knowledge 

1. checkout master branch 
2. copy and paste first patch set to a separate file 
3. edit commit log if required 
4. request for patch review 
5. if not okay, update patch and goto 4
6. if okay, apply patch to master branch 
7. commit to master branch 
8. if available copy and paste next patch set to a separate file and goto 3 

This is bascially what i did with mostly of your first windows patch some years ago :-)  


Comment 4 Tor Lillqvist 2009-11-23 10:59:22 UTC
Hmmm... you seem to be still seeing this in a SVN view;) I have a git *clone* (not a "checkout") of the upstream repository, into the master branch of which I have *committed* the individual commits from the dbus4win repo. That was the easiest and simplest way to do it. When I mailed the list, there were 114 commits in my local repository that hadn't been pushed to upstream (not that I even could push, I don't have rights). They already are individual commits ("patches") that can each be seaparately reviewed. Most of them are straight so-called cherry-picks (that's a git term, I used it in the specific git sense) from the dbus4win repository.

I am sorry, but the more I think of it, the less willing I am to start combinining these 114 commits (or patches) into a smaller number of larger patches. Too much work for too little gain.

I do see this whole situation a bit ridiculous. Years ago, in SVN times, when I first submitted an experimental patch for dbus on Windows (which did many things in wrong ways, I certainly admit), it was indeed a one large diff with unrelated changes to the same source files intermixed. This was obviously considered extremely hard to review.

Then, now this year, using git, the dbus4win people went through that patch and picked out changes that still actually make sense, and committed them as individual git commits to their repository. And then they did more work, again committing individual separate changes as separate commits. As one should in a git-based workflow, to the best of my understanding.

And the result? Now then individual git commits are required to be combined back into larger patches...
Comment 5 Ralf Habacker 2009-11-24 05:34:07 UTC
(In reply to comment #4)
> Hmmm... you seem to be still seeing this in a SVN view;) 
probably yes 

> I have a git *clone* > (not a "checkout") of the upstream repository, into the master branch of which
> I have *committed* the individual commits from the dbus4win repo. That was the
> easiest and simplest way to do it. When I mailed the list, there were 114
> commits in my local repository that hadn't been pushed to upstream (not that I
> even could push, I don't have rights). They already are individual commits
> ("patches") that can each be seaparately reviewed. 
> Most of them are straight so-called cherry-picks (that's a git term, I used it in the specific git sense) from the dbus4win repository.

Thanks for this explanation - it looks that git has all required features to perform the merge so that we can go the next steps in the merge which are documentated in http://dbus.freedesktop.org/doc/HACKING

Please file them at http://bugzilla.freedesktop.org under component
dbus, and also post to the mailing list for discussion.  The commit
rules are:

-> done

 - for fixes that don't affect API or protocol, they can be committed
   if any one qualified reviewer other than patch author
   reviews and approves

... 

The reviewer group that can approve patches: Havoc Pennington, Michael
Meeks, Alex Larsson, Zack Rusin, Joe Shaw, Mikael Hallendal, Richard
Hult, Owen Fraser-Green, Olivier Andrieu, Colin Walters, Thiago
Macieira, John Palmieri.

Who is willing to perform the review ? 

> (not that I even could push, I don't have rights). 

After patches are approved they need to be pushed - who will do this ? You or somebody else ? 




Comment 6 Ralf Habacker 2009-11-25 02:32:20 UTC
> Yeah, unfortunately the git commit messages in the dbus4win repository are not
> always as nice as one might hope. Some of them have very long lines (a result
> of the git client they used, I guess). 
<snip>
> one can't edit the commit message, 
<snip>
> Or is it possible?

I need some time to find a solution  from http://cworth.org/hgbook-git/tour/

2.7.6 Fixing up a broken commit (before anyone else sees it)

So now that we've cloned a local repository, made a change to the code, setup our name and email address, and made a careful commit, we're just about ready to share our change with the world. But wait, that commit message has that embarrassing misspelling in it. Wouldn't it be nice to touch that up before we post this commit with a never-to-be-changed again commit identifier?

This is the exact situation for which "git commit --amend" was invented. So you can just run that now and fix the broken commit message in the editor:

$ git commit --amend

-> This problem is fixed too 
Comment 7 Ralf Habacker 2009-11-25 08:20:57 UTC
(In reply to comment #6)
> > Yeah, unfortunately the git commit messages in the dbus4win repository are not
> > always as nice as one might hope. Some of them have very long lines (a result
> > of the git client they used, I guess). 
> <snip>
> > one can't edit the commit message, 
> <snip>
> > Or is it possible?

I just found a much easier way by using the git am command. 

git-am - Apply a series of patches from a mailbox 

The file created with the format-patch command has exactly the format the am command supports (mbox). 

Now to create a cleaned up merge of freedesktop and till's patch by 

1. checkout a fresh copy from freedesktop.org master branch 
2. edit the log messages in the appended patch 
3. run git am <patchfile> to commit all patches in the local copy 





Comment 8 Ralf Habacker 2009-11-25 08:44:52 UTC
(In reply to comment #5)

> Who is willing to perform the review ? 

Tor had already performed a review of the code by excluding problematic commits from the base source: 

He wrote in README.dbus4win: 

The following commits were left out because they either 0) were
already upstream, 1) seemed broken, or 2) seemed pointless to me,
especially if the commit message didn't say much. For each commit left
out, below is three lines: its id, the first log comment line, and an
explanation why I left it out.

f719d454329a1a54ff2d569feaf10ceee7cead50
configure.in: not all gccs support -Wno-pointer-sign
already upstream

6eddb6e1229b617ad44c8d61107983229dc7d589
configure.in: fail abstract socket test gracefully when cross-compilin
already upstream

c54dd9eefa5ed0d54be3b09cf30c12f05130fda1
r783: one ifdef lesser
probably pointless

93c288ca3e86e7760d3ac3fa6490257c1b6dc544
r783: compile if ENOMEM or EINTR is undefined.
huh, ENOMEM and EINTR is defined in the Microsoft C library

4f4ba13357da15c35a9d4ad111bf972a5d0d5db0
r783: introduced _dbus_daemon_release() function called in _dbus_loop_quit()...
this can't be right, there can be nested dbus loops afaik

124eb334324472a10de2c7e0a1455d353431f08f
unix build fixes against windbus-svn
can't be right to make a char* const when the code below then assigns through it

594bd19a0c45822d8763871e3491d99beb4d22cb
introduce DBUS_DIR_SEPERATOR and make use of it. On windows, disable some system service tests
"forward" slashes work fine on Windows, no need to uglify code with DBUS_DIR_SEPARATOR

cd2cdb3457f72cf058d5b19796ee691ceca6b52c
r783: introduced DBUS_CLEANUP_OLD_SERVICES that does cleanup prev zombie-instances...
unsure about this. something specific to KDE-on-Windows?

eeedba5b2f5b008819619764943caddd52441adf
build fixes for OS X, xcode 3.1.2, gcc 4.2.1
doesn't apply, seems to be upstream?

7dc7f71cf3003e006f6e177b5460d14a7cdbf2de
configure.in: fix pthread detection
doesn't apply

05b37fa87b1f6aa670de9007879f53a8198a7a89
configure.in: suppress -fPIC, -fPIE for W32
doesn't apply

b0d14fed92d9f539cd3478e72dc2f3ea75f3941a
configure.in: only check for wspiapi.h on Windows
nah, no harm checking for it on UNIX too

1724fc10e516e2d570537d522d8608abba90be5e
prototypes cleanup
related to cd2cdb3457f72cf058d5b19796ee691ceca6b52c which was skipped above



Comment 9 Ralf Habacker 2009-11-25 09:11:41 UTC
(In reply to comment #8)

+4f4ba13357da15c35a9d4ad111bf972a5d0d5db0
+r783: introduced _dbus_daemon_release() function called in _dbus_loop_quit()...
+this can't be right, there can be nested dbus loops afaik

_dbus_daemon_release() is indented to release the shared memory area holding the connection parameters for the session bus. 

When the recent implementation is wrong I guess dbus_daemon_release should be called on leaving the last loop as shown below: 

_dbus_assert (loop->depth > 0);  
  
  loop->depth -= 1;

  if (loop->depth == 0)
    _dbus_daemon_release ();

  _dbus_verbose ("Quit main loop, depth %d -> %d\n",
                 loop->depth + 1, loop->depth);
}

Comment 10 Ralf Habacker 2009-11-27 06:02:08 UTC
(In reply to comment #5)
> Who is willing to perform the review ? 

There is a compile problem on mingw 
 
D:\daten\kde\mingw-root\build\contributed\dbus-freedesktop-src-trunk\work\mingw-Debug-svnHEAD>mingw32-make
[ 49%] Built target dbus-1
[ 50%] Built target dbus-test
[ 66%] Built target bus-test
[ 82%] Built target dbus-daemon
[ 83%] Built target shell-test
[ 84%] Built target spawn-test
[ 85%] Built target test-exit
[ 86%] Built target test-names
[ 87%] Built target test-segfault
Linking C executable ..\bin\test-service.exe
CMakeFiles\test-service.dir\D_\daten\kde\download\svn-src\dbus-freedesktop-src\test\test-service.obj: In function `handle_delay_echo':
D:/daten/kde/download/svn-src/dbus-freedesktop-src/test/test-service.c:237: undefined reference to `usleep'
collect2: ld returned 1 exit status
mingw32-make[2]: *** [bin/test-service.exe] Error 1
mingw32-make[1]: *** [test/CMakeFiles/test-service.dir/all] Error 2
mingw32-make: *** [all] Error 2

a related patch is listed below 
-----------------------------------------------------------------------------
diff --git a/test/test-service.c b/test/test-service.c
index ee0086c..1688974 100644
--- a/test/test-service.c
+++ b/test/test-service.c
@@ -234,8 +234,8 @@ handle_delay_echo (DBusConnection     *connection,

   _dbus_verbose ("sleeping for a short time\n");

-  usleep (50000);
-
+  _dbus_sleep_milliseconds(50);
+
   _dbus_verbose ("sending reply to DelayEcho method\n");

   dbus_error_init (&error);
-----------------------------------------------------------------------------

Comment 11 Ralf Habacker 2009-11-27 06:13:15 UTC
(In reply to comment #5)

more reviews

set DBUS_TEST_DATA=D:\daten\kde\download\svn-src\dbus-freedesktop-src\test\data

D:\daten\kde\mingw-root\build\contributed\dbus-freedesktop-src-trunk\work\mingw-Debug-svnHEAD>bin\dbus-test
Test data in D:\daten\kde\download\svn-src\dbus-freedesktop-src\test\data
dbus-test: running string tests
dbus-test: checking for memleaks
dbus-test: running sysdeps tests
dbus-test: checking for memleaks
dbus-test: running data-slot tests
dbus-test: checking for memleaks
dbus-test: running misc tests
dbus-test: checking for memleaks
dbus-test: running address tests
dbus-test: checking for memleaks
dbus-test: running server tests
6492: assertion failed "(real) != NULL" file "D:\daten\kde\download\svn-src\dbus-freedesktop-src\dbus\dbus-string.c" line 468 function _dbus_string_get_const_da
ta
Backtrace:
3       NtWaitForSingleObject+0x15
3       WaitForSingleObject+0x12
3       dbus_delete_file+0x527
3       dbus_print_backtrace+0x10
3       dbus_abort+0xb
3       dbus_real_assert+0x44
3       dbus_string_get_const_data+0x46
3       dbus_delete_file+0x66
3       dbus_noncefile_delete+0x6d
3       dbus_server_test+0x27f
3       dbus_server_unref+0x26e
3       dbus_server_test+0x1af
3       dbus_sysdeps_test+0x478
3       dbus_internal_do_not_use_run_tests+0xf9
2       dbus-test.exe+0x1374
2       dbus-test.exe+0x124b
2       dbus-test.exe+0x1298
3       BaseProcessInitPostImport+0x8d


Comment 12 Ralf Habacker 2009-11-27 06:35:51 UTC
(In reply to comment #5)

a link review using msvc builds

Linking C shared library ..\bin\dbus-1d.dll
<snip>
dbus-1d.def : error LNK2001: Nicht aufgel÷stes externes Symbol "_dbus_change_identity".
dbus-1d.def : error LNK2001: Nicht aufgel÷stes externes Symbol "_dbus_counter_adjust".
dbus-1d.def : error LNK2001: Nicht aufgel÷stes externes Symbol "_dbus_counter_get_value".
dbus-1d.def : error LNK2001: Nicht aufgel÷stes externes Symbol "_dbus_get_install_root".
dbus-1d.def : error LNK2001: Nicht aufgel÷stes externes Symbol "_dbus_getgid".
dbus-1d.def : error LNK2001: Nicht aufgel÷stes externes Symbol "_dbus_getsid".
dbus-1d.def : error LNK2001: Nicht aufgel÷stes externes Symbol "_dbus_getuid".
dbus-1d.def : error LNK2001: Nicht aufgel÷stes externes Symbol "_dbus_message_add_size_counter".
dbus-1d.def : error LNK2001: Nicht aufgel÷stes externes Symbol "_dbus_message_add_size_counter_link".
dbus-1d.def : error LNK2001: Nicht aufgel÷stes externes Symbol "_dbus_message_remove_size_counter".
dbus-1d.def : error LNK2001: Nicht aufgel÷stes externes Symbol "_dbus_user_at_console".
..\bin\dbus-1d.lib : fatal error LNK1120: 11 nicht aufgel÷ste externe Verweise.
LINK Pass 1 failed. with 1120


Comment 13 Ralf Habacker 2009-11-27 06:46:22 UTC
(In reply to comment #8)


> cd2cdb3457f72cf058d5b19796ee691ceca6b52c
> r783: introduced DBUS_CLEANUP_OLD_SERVICES that does cleanup prev
> zombie-instances...
> unsure about this. something specific to KDE-on-Windows?

This feature was introduced by Jaruslav Staniek to windbus in late 2008. Unfortunally without any detailed description about the reasons. 

From the source I get the impression that this feature cares about removing registrated services when the related process was dead or has been killed. 

Unfortunally the patch defines a platform related function called _dbus_process_exists() in bus/services.c, which probably belongs to a windows specific file. 
Comment 14 Ralf Habacker 2009-11-29 10:14:11 UTC
(In reply to comment #5)
> > (not that I even could push, I don't have rights). 
> 
> After patches are approved they need to be pushed - who will do this ? You or
> somebody else ? 

I checked my freedesktop git account - I'm technical able to commit patches to dbus master branch. I verified this by pushing the patch set 001. 


Comment 15 Ralf Habacker 2009-11-29 10:38:36 UTC
(In reply to comment #5)
> Who is willing to perform the review ? 
> 
more reviews: 

4f0b78ae7e2948621f6ba582957bb6908e19285f
[PATCH 001/114] mingw compile++ , w2k-support++
windows only, uncritical -> committed 

e7c42374c265006ada3ba68d03019a642e45f1c7
[PATCH 002/114] msvc 8 compile++ and removed some stupid casts
windows only, uncritical -> committed 

0442a522fbcb775cbda9def19592e71495d49daa
[PATCH 003/114] Include <stdlib.h> for envion on Windows
patches common file ->should use global platform definition DBUS_WIN instead of _WIN32 -> committed with little modifications

0442a522fbcb775cbda9def19592e71495d49daa
[PATCH 003/114] Include <stdlib.h> for envion on Windows
patches common file ->should use global platform definition DBUS_WIN instead of _WIN32 -> committed with little modifications

aae7289bb0901a81f1b88ecef5cf20de3af25213
[PATCH 004/114] dbus/dbus-sysdeps-win.c: fix linking with mingw32
windows only, uncritical -> committed 

From cd90173c01db2b9022141b5a0264de1eda438d7b Mon Sep 17 00:00:00 2001
[PATCH 005/114] hide console when autostarting dbus-daemon
windows only, uncritical -> committed 

From dacfa05886f2a160b8c07a426ee2801336ac738a Mon Sep 17 00:00:00 2001
PATCH 006/114] remove an assert for windows only - don't know why this was added...
windows only, uncritical -> committed 


Comment 16 Ralf Habacker 2009-11-30 01:36:20 UTC
(In reply to comment #15)
> (In reply to comment #5)
> > Who is willing to perform the review ? 
> > 
> more reviews: 
> 

73bb04ec3d73e56cd1c9dd881c0175fcd7937954
[PATCH 008/114] use dbus_watch_get_socket in dbus_watch_get_unix_fd on windows
clean -> committed

7ca4e932e6661349701b53c77d50f2e8531bed5f Mon Sep 17 00:00:00 2001
[PATCH 009/114] no fake lock under Windows
clean -> committed

b97d2de87b2d28ab10fda9dd497c031a4d6ab74d
[PATCH 010/114] patch from Thorvald Natvig to fix a copy+paste error that could result in a dangling handle at windows. 
clean -> committed

2ac6738799232ebb1ae49f3fc807bea1254cd121
[PATCH 011/114] all changes for dbus-1.2 API
clean -> committed
Comment 17 Ralf Habacker 2009-11-30 01:51:07 UTC
(In reply to comment #15)
> more reviews: 


2232bfcc39b6d75a78ca5241904f7d501cb952cb
[PATCH 014/114] build fixes for tests on unix
clean -> committed

2fbdc8830a8b9b8097c8eb0cddb06c531fea22f0
[PATCH 015/114] set -DDBUS_BUILD_TESTS
clean -> committed

5f005355aa6a04ec4d02610af8ed6ca0525c0103
[PATCH 016/114] fix configure check and VA_COPY usage
clean -> committed

f16a14f1f5835e9339dac1676bac14a729350832
[PATCH 017/114] do not call functions with side-effects inside assert
clean -> committed

b63d39d793aa83da4b481e5f07bd4b2a2e585f37
[PATCH 018/114] be more verbose when locking the mutex fails
clean -> committed

4d8bc1337ef9878d4a5d66dd04b280d0b635e5d4
[PATCH 019/114] add check for abstract sockets
clean -> committed

Comment 18 Ralf Habacker 2009-11-30 02:32:27 UTC
(In reply to comment #15)

> more reviews: 

cd8944b6d21c733b68b150c16825aab33df03e8c
[PATCH 020/114] don't leak string list in error case
clean -> committed

db95356711efd0e92dfe541f6d925bf69eeaf516
[PATCH 021/114] use correct test data dir
clean -> committed

3393302ffb9ce3521e564c83dc458dbcc1553d53
[PATCH 022/114] use CMAKE_BUILD_TYPE to differ between debug and release.
clean -> committed

5984b0b8dd0fcfe6bfe7e15e2c535cbaf9d582c8
[PATCH 023/114] generate all config files from the .in files. Set some of the required variables replaced in the .in files by configure_file
clean -> committed

a7d0b98cd9ecec2278d52d16b61ec791dd59c24a
[PATCH 024/114] do not add d suffix on non-windows
clean -> committed

dc1a691e2a6854361571b522c67609e656d4a5a8
[PATCH 025/114] build dbus-daemon-launch-helper-test and friends
clean -> committed

206727c78bfc451515c9ea07138428d67454298d
[PATCH 026/114] cleanup: these files are already generated elsewhere
files in cmake/data/test are now obsolate and has been removed too -> committed

TODO: check if (d)bus-test can really use the pipe implementation on windows and if the unix listen statement in test/data/debug-allow-all-*.conf.in is ignored. If not, this parameters has to be customized on unix and windows

f79c417f58d78bec8a5a09ee135b447ca0d40873
[PATCH 027/114] rename second test-names to test-names2 to avoid confusion
clean -> committed

5428d8a456f92c5c5ff9912755e7a61208b9e0ab
[PATCH 028/114] fix test and binary names
clean -> committed

9b0b17585301e3eebdb53b9dcfd8a86689b68734
[PATCH 029/114] rename test-spawn to spawn-test, to match autotools.
clean -> committed
Comment 19 Ralf Habacker 2009-11-30 02:43:46 UTC
(In reply to comment #15)
> more reviews: 

2db7d247df18425ab12db3b6205dd7dd0bf5f768
[PATCH 030/114] Use correct sources to build test-shell-service, fixes a hang in bus-test
clean -> committed

b6a821394f0aaafb44476f234edff7acccdaabfa
[PATCH 031/114] bus-test-launch-helper is a unit test, test-service and test-shell-service are not. Rename test-shell to shell-test to match autotools
clean -> committed

6b65f83973414da210ec0b7803da5614be72729e
[PATCH 032/114] disable launch-helper on windows for now until it builds
clean -> committed

ea4f1931725302a2e1454858f90714d90b67ab35
[PATCH 033/114] fix TEST_SOCKET_DIR on windows, do not override value from top-level in test/
clean -> committed

f92d211bfc40656a53377592d10dbdef7787ce38
[PATCH 034/114] define DBUS_BUILD_TESTS only if the option is enabled
clean -> committed

Comment 20 Ralf Habacker 2009-11-30 02:55:51 UTC
(In reply to comment #15)
> more reviews: 

The following patches affects the unix build system 

4ddbfa4e9ca08eef3b3759c1b9511ac6cf372300
[PATCH 035/114] WIN32: add versioninfo, and export symbols

bfa3c0a93f14275ff1d91ea0df3371baf3684d50
[PATCH 036/114] configure.in: add W32 extra libs

9f17adf989b603564b143a6876a77afd5a4af539
[PATCH 037/114] configure.in: don't look for X11 on W32

93e19170d2b0de12b924abf50b7c327be4f3ebfe
[PATCH 038/114] Makefile.am's: reorder libraries so static libs come first (fixes linking on W32)

a34745e22a91be46e1e26ef78463a6155ecabe97
[PATCH 039/114] tools/Makefile.am: conditional compilation for W32

Is anyone here who could check if these patches do not break something on unix ? 


Comment 21 Ralf Habacker 2009-11-30 03:01:48 UTC
(In reply to comment #15)
>
> more reviews: 
> 
8a7a80c8c1bdac4a438ca81de118c5e03a098dfa
[PATCH 041/114] fix warning, use Sleep, not _sleep on windows
clean -> committed

cd8ef58d9cb14a2b5c7aa7a8fca642f185478940
[PATCH 042/114] tools/dbus-launch-win.c: fix typo
clean -> committed

ff07bf46a07247837839e162c473958cb201473c
[PATCH 043/114] dbus/dbus-sysdeps-util-win.c: DBUS_WIN is always defined here
clean -> committed

a530dcfccef3bf8128d150fa376a9fabb9a9ef49
[PATCH 044/114] Fix DBusMessage compiler warnings by adding some missing casts.
clean -> committed

ec5d95c9dc7c55f31f09df7e052106a0b5981c19
[PATCH 045/114] tools/dbus-launch-win.c: strrchr -> _mbsrchr
clean -> committed

881b51653c2f8c50a0a1f75532dc494ef884dfea
[PATCH 046/114] Only define _WIN32_WINNT if not already defined.
clean -> committed

67fa8bbbd3af9ae104528f09ad906211ad0bb61e
[PATCH 047/114] Add missing stdio.h include needed for _vsnprintf().
clean -> committed

67f86293e72ef6c22188eaf41e96778bc9bd6b75
[PATCH 048/114] Remove a misleading comment.
clean -> committed

c79b7371b6f079dbad5f697570ddd23e8c3962db
[PATCH 049/114] Use CreateDirectory() instead of complicating things.
clean -> committed
Comment 22 Ralf Habacker 2009-11-30 03:23:20 UTC
(In reply to comment #15)
> more reviews: 

923024ecee752ef1ab61ee50204a4c8ae8641fe1
[PATCH 050/114] Fix broken Windows implementation of _dbus_printf_string_upper_bound().
clean -> committed

5351d5456c89adddf7c8c0622f535a92718409cd
[PATCH 051/114] dbus/dbus-connection.c: use dbus_message_type_to_string instead of printing the naked message type
clean -> committed

f4840e63b582f679f7c57b5cfedc55c5a665c08a
[PATCH 052/114] tools/dbus-launch-win.c: TODO++
clean -> committed

28f160063b11adaf10f9d64d26f56aec19fabb94
[PATCH 053/114] dbus/dbus-transport.c: fix {our,auth}_identity mismatch
did not apply and need manual patching -> committed

f54a706607ec51a30d9e02fdf9ea20026e99954c
[PATCH 054/114] dbus/dbus-transport.c: _dbus_credentials_get_windows_sid might return NULL
clean -> committed

0bc3e8d60350f3bdb414e8788ce7f7d8237af6fc
[PATCH 055/114] dbus/dbus-internals.h: 2x const char foo[] -> const char foo *
clean -> committed
Comment 23 Ralf Habacker 2009-11-30 03:24:10 UTC
*** Bug 25207 has been marked as a duplicate of this bug. ***
Comment 24 Ralf Habacker 2009-11-30 04:17:59 UTC
(In reply to comment #15)
> more reviews: 

1669a64b67895724c9c5ffc9f461041c796b1af1
[PATCH 007/114] _dbus_verbose_real: (optionally) use OutputDebugString()
renamed cpp define to fix dbus naming scheme and added related cmake option -> committed 

99ee9a061bfa36277d9b3812b325ca49d0531d50
[PATCH 056/114] dbus/dbus-internals.c: "#define inline" is only needed on MSVC, not mingw
clean -> committed

ae9bd5798c4937bbd658ee38dfc487d847e47d1d
[PATCH 057/114] dbus/dbus-spawn-win.c: implement missing _dbus_babysitter_get_child_exit_status
clean -> committed

f2481044d403e281bc179328537c2f504d9df883
[PATCH 058/114] dbus/dbus-sysdeps-win.h: add _dbus_win_get_dll-module
clean -> committed

457a6c0f95dedde144f6769f8cd976e63bd77fcc
[PATCH 059/114] dbus/dbus-sysdeps-win.c: #define socklen_t -> typedef
clean -> committed
Comment 25 Ralf Habacker 2009-11-30 04:34:02 UTC
(In reply to comment #15)

> more reviews: 

04ffd8e4c33c374618518ea5cd998de9bfa7a239
[PATCH 060/114] dbus/dbus-sysdeps-*win.c: remove DBusFile abstraction
this require to remove some symbols from the def file -> committed

9e00cca46fdd4cee0ee0f91249d1f208c78c3068
[PATCH 061/114] dbus/dbus-sysdeps-win.c: Use CryptoApi to get random numbers
clean -> committed

7b21f5ef8c12ef5bd9a005c593336659d845b79d
[PATCH 062/114] dbus/dbus-sysdeps-win.c: set an error when _close fails in _dbus_pipe_close
clean -> committed

4b5efaddba36e94291c02d8eea536d1182aa83d8
[PATCH 063/114] dbus/dbus-sysdeps-win.c: _dbus_getpid() returns dbus_pid_t
clean -> committed

e57bc4f3d0de4d85b00cc209a3b8488622ea07d8
[PATCH 064/114] dbus/dbus-sysdeps-win.c: use GetTempPath, not getenv, in _dbus_get_tmpdir
clean -> committed

fad2ed39e09a82a89df03a0d7c570d172063c4fb
[PATCH 065/114] dbus/dbus-sysdeps-win.c: use MoveFileEx, not unlink+rename, in _dbus_string_save_to_file
clean -> committed

77f448f1a489ab2bc58934d0afb922ecb45e3b07
[PATCH 066/114] dbus/dbus-sysdeps-win.c: add _dbus_win_error_string, and use after MoveFileEx instead of _dbus_error_from_errno
clean -> committed

1d13153582e53dc5527f22f393c656134f2710e9
[PATCH 067/114] dbus/dbus-sysdeps-win.h: move declarations into .c (used nowhere else)
clean -> committed

27dba2cf43c8184be65fdfd35b138efbb95e8c84
[PATCH 068/114] dbus/dbus-sysdeps-*win.c: remove #undef open, which has no effect
clean -> committed

83f34795d3a52d879fa11a1029677ffff7345a57
[PATCH 069/114] dbus/dbus-sysdeps-util-win.c: remove unused str*_s definitions
clean -> committed
Comment 26 Ralf Habacker 2009-11-30 23:33:45 UTC
(In reply to comment #15)
> more reviews: 

f93d4de32c0a75e18e98774ea37983a83f0dc897
[PATCH 070/114] dbus/dbus-sysdeps-win.c: tighter "scoping" for alternate _dbus_poll implementations
clean -> committed

a0f3c78ef799aea51cbd472c8740d05a172b7b2b
[PATCH 071/114] dbus/dbus-sysdeps-util-win.c: use GetFileAttributes instead of CreateFile in _dbus_file_exists
clean -> committed

b9d04c1c503c0cb740e45638b614f6884f5dc396
[PATCH 072/114] dbus/dbus-sysdeps-win.c: _dbus_windows_user_is_process_owner belongs to -util-win.c
clean -> committed

1214fc95bc193cc804c9b5ff0037043784933d34
[PATCH 073/114] remove duplicate dbus_babysitter_get_child_exit_status impl
already applied -> skipped 

a5b172fd8965a19fae5e9c3731851214cd4ae7ff
[PATCH 074/114] do not run test binaries as unit tests if they aren't unit tests
clean -> committed

ab8802d23481f57663208d7d287917ffcd9ff419
[PATCH 075/114] define _DEBUG for debug builds
clean -> committed

ffe2d0886ba2767b700a3f4e475e41fa7180777f
[PATCH 076/114] _dbus_get_install_root assumes that dbus-daemon is in a bin/ subdirectory. That's not a common directory structure on windows, so weaken the assumption:
clean -> committed

88ff6e09d32f2be4e5351ca3a377032602a3668b
[PATCH 077/114] msvc build fix: update .def
already applied -> skipped 


After this patches I did a break and tried to get the test applications running. I recognized the following issues: 

1. <build-dir>/test/data/valid-config-files/debug-allow-all*.conf are broken on windows (the related *.in files contains unsupported listen protocol ->unix:)
For this special *.cmake files are used as workaround. A complete fix would be to cutomize the *.in files to support a TEST_LISTEN build system variable -> need to fix autotools build system. 

2. A short test shows that the debug-pipe implementation seems is broken 

It may be that following patches may fix this issues, so further tests are skipped. 

Comment 27 Ralf Habacker 2009-11-30 23:39:29 UTC
4ddbfa4e9ca08eef3b3759c1b9511ac6cf372300
[PATCH 035/114] WIN32: add versioninfo, and export symbols
clean -> committed

bfa3c0a93f14275ff1d91ea0df3371baf3684d50
[PATCH 036/114] configure.in: add W32 extra libs
clean -> committed

9f17adf989b603564b143a6876a77afd5a4af539
[PATCH 037/114] configure.in: don't look for X11 on W32
clean -> committed

93e19170d2b0de12b924abf50b7c327be4f3ebfe
[PATCH 038/114] Makefile.am's: reorder libraries so static libs come first (fixes linking on W32)
clean -> committed

a34745e22a91be46e1e26ef78463a6155ecabe97
[PATCH 039/114] tools/Makefile.am: conditional compilation for W32
clean -> committed

fa636eb006022af6c5a979fa8779c97582ab10e1
[PATCH 040/114] bus/Makefile.am: conditional compilation for W32, use EXEEXT in install hooks
clean -> committed

f356fc1c8f26deb0d38d004ed4b8917a4cfdf25c
[PATCH 078/114] configure.in: do not define DBUS_UNIX unconditionally
clean -> committed

7d078b5bd188737580df1cd49fc5b16c49f99089
[PATCH 079/114] remove dbus-uuidgen from Windows build
clean -> committed

71a1f8ca66bf6c61d806e14487934fbdc79300fb
[PATCH 080/114] use WINDRES instead of RC to compile rc file
clean -> committed

643351983b7a69293b8d88ad7a8050c50eedfec5
[PATCH 081/114] Move some functions from dbus-sysdeps-util-win.c to dbus-sysdeps-win.c
clean -> committed

5e0a4f767db314dcaffdc2b1166640a91b3757d7
[PATCH 082/114] -util.c doesn't belong into the lib
clean -> committed

03055aa8343a0ebf7dadc1d9980068028f56fb8d
[PATCH 083/114] bus/Makefile.am: make dbus-daemon and friends bin_PROGRAMS on Windows
clean -> committed


Comment 28 Ralf Habacker 2009-12-01 00:56:56 UTC
d7b7502cfe1db64f9599161762ea2313441ddf0d
[PATCH 084/114] The current state of the nonce-tcp implementation

f3ed9764b232a418e703411080b4d5c68725418a
[PATCH 085/114] Cleanup of nonce code

c58829f198dcd76fede10cc1e9f14074860728d0
[PATCH 086/114] Improve error handling in nonce code

69b8532e742af497ef3299e6a0ea737e59e0cfc2
[PATCH 087/114] Add nonce-tcp section to the specification (draft)

d15f4d0bd20d14c129ee1981d0b72c90e11f5779
[PATCH 088/114] Fixes to the nonce code

e2f810808544d157c995c674f35cbdb4ca7e4a33
[PATCH 089/114] Add api dox for nonce-tcp

a5042dd2566e02bbcdb079c4c6dfec8d3cf5b397
[PATCH 090/114] Fix IPv6 setup (the default on Vista)

b52508439866e5295f34a770546e5d01e0a041e9
[PATCH 091/114] Fixes to the nonce code

-> committed 
Comment 29 Ralf Habacker 2009-12-01 01:34:32 UTC
(In reply to comment #15)

> more reviews: 

09ab1c0f2ef40cbb4866e8c41e4f1ad29ae97671
[PATCH 092/114] with msvc, replace va_copy by assigning the va_lists
clean -> committed

bd1a545d57ba62e9d9fa067db9e5fd0deb3273b4
[PATCH 093/114] Don't bother with -Werror on Windows for now
clean -> committed

2d0e19318edc39b4fb1a24435692a9d75bc7212d
[PATCH 094/114] Use multi-byte string functions on Windows
clean -> committed

541fbbc87da5ccf9eb6ff4e2eaa087f9b699f17c
[PATCH 095/114] Look for config files also in the normal Unix-style location
clean -> committed

9b246c1db774d4e36613dbc6be0bed6219d91875
[PATCH 096/114] Rename README.win to README.windbus to match its origin
clean -> committed

d028bf6f3b6764147961ba7c865a4d19153dda1b
[PATCH 097/114] Tell where the file comes from and that it might not be reliable
clean -> committed

091c4d6dea58ee486eb296bdebceed556e84d33d
[PATCH 098/114] Add file describing the merge of the dbus4win code
clean -> committed

6f70e6ae4f0f303070eedd622b3a607df0ae58b1
[PATCH 099/114] Don't use -fPIC and -fPIE on Windows
clean -> committed

82af675d2ea52c82a88a6b44ef7503ee1647c389
[PATCH 100/114] Don't use DBUS_DIR_SEPARATOR
clean -> committed

d8cdc5f02d33049e52aa62f9d6f0b34489fbdead
[PATCH 101/114] Add a dbus/dbus-1.def file
cmake/dbus/dbus-1.def.cmake
clean -> committed

22b47ae8e1ed005c5e6c95947750e54a73ce4825
[PATCH 102/114] Drop internal symbols, add missing symbols
internal symbols are required to bus-test, which is fixed in an additional commit -> committed

150cde7f0dac24658378d98eb3c32989f8e01194
[PATCH 103/114] Use dbus-1.def on Windows
clean -> committed -> added additional commit to use this file for cmake too

04bf887b5d03368279990d70a28c64150ec9a642
[PATCH 104/114] Actually do install the .def file on Windows
clean -> committed

7e9de96f4621253ec626e41401085ee252dca085
[PATCH 105/114] DBUS_VERSION is always three numbers
clean -> committed

b6d8418b1bee040282fd835ec2e462e15b6ab5dc
[PATCH 106/114] Check for dirent.h
clean -> committed

a9f08aba22d3858126f22d9fbe8073d1118a9378
[PATCH 107/114] Avoind warnings on Windows
spelling in comment fixed -> committed

2cf40b2376c50a48f7d3a5e9f581557623cdcb8a
[PATCH 108/114] Use nonce-tcp default session bus on Windows
not sure yet if this should be really the default -> committed

18f7df4f5f2267eed70acdca813a20744fb19206
[PATCH 109/114] Drop terminating slash in _dbus_get_tmpdir
clean -> committed

d22c5c4a20a3efc40d0dd5e1ff47921728e3c2b6
[PATCH 110/114] Windows fixes
clean -> committed

140dc3c49e1b53566350d18a205182e30caa547c
[PATCH 111/114] Handle also WinSock errors in _dbus_error_from_errno
clean -> committed

400dfb67c6e2715df0410e3d9984f348c480cbe1
[PATCH 112/114] Don't fake network errno values on Windows
clean -> committed

c26a0beff6173532fd1937c1c3edc05f156c8305
[PATCH 113/114] Set the DBusError
clean -> committed

f11d42044bb6dceecaf1a5d68ac08efd0314b297
[PATCH 114/114] Drop unused complex function
clean -> committed

@Tor: now patches are applied and further refactoring/bug fixing could be done




Comment 30 Ralf Habacker 2009-12-01 01:38:32 UTC
(In reply to comment #9)
> (In reply to comment #8)
> 
> +4f4ba13357da15c35a9d4ad111bf972a5d0d5db0
> +r783: introduced _dbus_daemon_release() function called in
> _dbus_loop_quit()...
> +this can't be right, there can be nested dbus loops afaik
> 
> _dbus_daemon_release() is indented to release the shared memory area holding
> the connection parameters for the session bus. 
> 
> When the recent implementation is wrong I guess dbus_daemon_release should be
> called on leaving the last loop as shown below: 
> 
> _dbus_assert (loop->depth > 0);  
> 
>   loop->depth -= 1;
> 
>   if (loop->depth == 0)
>     _dbus_daemon_release ();
> 
>   _dbus_verbose ("Quit main loop, depth %d -> %d\n",
>                  loop->depth + 1, loop->depth);
> }
> 

this fix is obsolate because bus\main.c contains already the required fix: 

  if (is_session_bus)
    _dbus_daemon_unpublish_session_bus_address ();



Comment 31 Ralf Habacker 2009-12-01 03:07:08 UTC
After a few additional fixes I got a connection to a freshly compiled dbus daemon from a qdbus client with version 1.2.4 by using the tcp procotol:

> start bin\dbus-daemon --session 

> set DBUS_SESSION_BUS_ADDRESS=tcp:host=localhost,port=12434
qdbus 
-----------------------------------------------
:1.2
org.freedesktop.DBus
-----------------------------------------------


> qdbus org.freedesktop.DBus /
-----------------------------------------------
method QString org.freedesktop.DBus.Introspectable.Introspect()
method void org.freedesktop.DBus.AddMatch(QString)
method QByteArray org.freedesktop.DBus.GetAdtAuditSessionData(QString)
method QByteArray org.freedesktop.DBus.GetConnectionSELinuxSecurityContext(QString)
method uint org.freedesktop.DBus.GetConnectionUnixProcessID(QString)
method uint org.freedesktop.DBus.GetConnectionUnixUser(QString)
method QString org.freedesktop.DBus.GetId()
method QString org.freedesktop.DBus.GetNameOwner(QString)
method QString org.freedesktop.DBus.Hello()
method QStringList org.freedesktop.DBus.ListActivatableNames()
method QStringList org.freedesktop.DBus.ListNames()
method QStringList org.freedesktop.DBus.ListQueuedOwners(QString)
signal void org.freedesktop.DBus.NameAcquired(QString)
method bool org.freedesktop.DBus.NameHasOwner(QString)
signal void org.freedesktop.DBus.NameLost(QString)
signal void org.freedesktop.DBus.NameOwnerChanged(QString, QString, QString)
method uint org.freedesktop.DBus.ReleaseName(QString)
method void org.freedesktop.DBus.ReloadConfig()
method void org.freedesktop.DBus.RemoveMatch(QString)
method uint org.freedesktop.DBus.RequestName(QString, uint)
method uint org.freedesktop.DBus.StartServiceByName(QString, uint)
-----------------------------------------------

> qdbus org.freedesktop.DBus /  org.freedesktop.DBus.GetId
-----------------------------------------------
7cf773f5ae9c94b6792b62db4b14f5fb
-----------------------------------------------

Comment 32 Ralf Habacker 2010-03-26 14:14:04 UTC
This work is finished now - thanks to all contributors for their really good work. Additional issues/todo could be handled in a different bug. 

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.