D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Ralf Habacker]

habacker added a comment.


  This review was superseded by D9669 

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks, michaelh, ngraham, bruns

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Ralf Habacker]

habacker abandoned this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Ralf Habacker]

habacker added a comment.


  https://phabricator.kde.org/D9480 is for kdbusaddons repo, but we need at 
least one review for kio using the new function in all related places. If 
https://phabricator.kde.org/D9480 is submitted, I can discard all "... service 
file must not have any path prefix on Windows" for kio repo and open a new one 
containing all required changes or cancel all but one with the related update.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[David Faure]

dfaure added a comment.


  Let's discard this one and proceed with https://phabricator.kde.org/D9480 
instead.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Ralf Habacker]

habacker added a comment.


  ping

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Ralf Habacker]

habacker added a comment.


  see https://phabricator.kde.org/D9480

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Ralf Habacker]

habacker added a comment.


  All mentioned git repos are using KDBusAddons as dependency.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[David Faure]

dfaure added a comment.


  Can you check if all the frameworks where this would be needed, have 
kdbusaddons as a dependency? That would certainly be a better option indeed, we 
already have frameworks providing cmake macros, which keeps things more modular 
(and keeps ECM more generic for other cmake users...).

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Ralf Habacker]

habacker added a comment.


  In https://phabricator.kde.org/D9423#182022, @habacker wrote:
  
  > 1. install dbus service file
  
  
  installing the service file requires a KDE specific path, 
${KDE_INSTALL_DBUSSERVICEDIR}, so a KF5 related name seems to be more 
descriptive
  
function(kf5_generate_dbus_service_file _executable _name _path)
  set(_file ${CMAKE_CURRENT_BINARY_DIR}/${_name}.service)
  set(_template ${CMAKE_CURRENT_LIST_FILE}/KF5DBusServiceTemplate.in)
  set(NAME ${_name})
  if(WIN32)
  set(EXEC ${_executable})
  else()
  set(EXEC ${_path}/${_executable})
  endif()
  configure_file(${_template} ${_file})
  install(FILES ${_file} DESTINATION ${KDE_INSTALL_DBUSSERVICEDIR})
endfunction()
  
  where KF5DBusServiceTemplate.in looks as
  
[D-BUS Service]
Name=@NAME@
Exec=@EXEC@
  
  and may be better located in the kdbusaddons repo  instead of 
extra-cmake-modules ? What is prefered ?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[David Faure]

dfaure added a comment.


  Makes sense, I guess.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Ralf Habacker]

habacker added a comment.


  In https://phabricator.kde.org/D9423#181793, @dfaure wrote:
  
  > A single .in file sounds good indeed. Sorry for approving the first one too 
fast.
  
  
  That means the solution needs to be applied to more then one git repo
  
  - kio
  - kwallet
  - just recgonized that kded is also affected 
(https://cgit.kde.org/kded.git/tree/src/org.kde.kded5.service.in)
  
  Would it then not be better to create and use an ecm function  for that ?
  
  e.g  something like
  
  function(ECM_GENERATE_DBUS_SERVICE_FILE  target)
  
  1. detect platform
  2. if windows
  3. set empty path
  4. else
  5. get target install location
  6. endif
  7. generate dbus service file from template contained in ecm packages or 
using raw write
  8. install dbus service file
  
  endfunction()
  
  where 'target' - is the cmake target name
  
  e.g.
  
  ecm_generate_dbus_service_file(kded)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[David Faure]

dfaure added a comment.


  A single .in file sounds good indeed. Sorry for approving the first one too 
fast.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Ralf Habacker]

habacker added a comment.


  In https://phabricator.kde.org/D9423#181574, @kfunk wrote:
  
  > In https://phabricator.kde.org/D9423#181460, @habacker wrote:
  >
  > > In https://phabricator.kde.org/D9423#181442, @kfunk wrote:
  > >
  > > > And I agree. There shouldn't be a need to use two different input 
files. That's the whole point of `configure_file(...)`: the interpolation of 
values happens *inside* the input files.
  > >
  > >
  > > This would be possible by using in the service file something like this
  > >
  > > [D-BUS Service]
  > >  Name=org.kde.kiod5
  > >  Exec=@SOME_PREFIX@kiod5
  >
  >
  > Sounds good.
  >
  > In the top-level CMakeLists.txt maybe:
  >
  >   if(WIN32)
  >set(DBUS_LIBEXECDIR)
  >   else()
  >set(DBUS_LIBEXECDIR ${KDE_INSTALL_FULL_LIBEXECDIR}/kf5/)
  >   endif()
  >
  
  
  The service template will then look like 
  [D-BUS Service]
  Name=org.kde.kioexecd
  Exec=@DBUS_LIBEXECDIR@kiod5
  
  just saw that for kcookiejar5 another variable is required and the above 
mentioned cmake fragment needs to be extended to
  
if(WIN32)
   set(DBUS_LIBEXECDIR)
   set(DBUS_BINDIR)
else()
   set(DBUS_LIBEXECDIR ${KDE_INSTALL_FULL_LIBEXECDIR}/kf5/)
   set(DBUS_BINDIR ${CMAKE_INSTALL_PREFIX}/bin/)
endif()
  
  and src/ioslaves/http/kcookiejar/org.kde.kcookiejar5.service.in looks like 
shown below
  [D-BUS Service]
  Name=org.kde.kcookiejar5
  Exec=@DBUS_BINDIR@kded5

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Aleix Pol Gonzalez]

apol added a comment.


  In https://phabricator.kde.org/D9423#181574, @kfunk wrote:
  
  > In https://phabricator.kde.org/D9423#181460, @habacker wrote:
  >
  > > In https://phabricator.kde.org/D9423#181442, @kfunk wrote:
  > >
  > > > And I agree. There shouldn't be a need to use two different input 
files. That's the whole point of `configure_file(...)`: the interpolation of 
values happens *inside* the input files.
  > >
  > >
  > > This would be possible by using in the service file something like this
  > >
  > > [D-BUS Service]
  > >  Name=org.kde.kiod5
  > >  Exec=@SOME_PREFIX@kiod5
  >
  >
  > Sounds good.
  >
  > In the top-level CMakeLists.txt maybe:
  >
  >   if(WIN32)
  >set(DBUS_LIBEXECDIR)
  >   else()
  >set(DBUS_LIBEXECDIR ${KDE_INSTALL_FULL_LIBEXECDIR}/kf5/)
  >   endif()
  >
  >
  > ... and then use that everywhere needed.
  
  
  +1

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: apol, kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Kevin Funk]

kfunk added a comment.


  @dfaure Opinions?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Kevin Funk]

kfunk added a comment.


  In https://phabricator.kde.org/D9423#181460, @habacker wrote:
  
  > In https://phabricator.kde.org/D9423#181442, @kfunk wrote:
  >
  > > And I agree. There shouldn't be a need to use two different input files. 
That's the whole point of `configure_file(...)`: the interpolation of values 
happens *inside* the input files.
  >
  >
  > This would be possible by using in the service file something like this
  >
  > [D-BUS Service]
  >  Name=org.kde.kiod5
  >  Exec=@SOME_PREFIX@kiod5
  
  
  Sounds good.
  
  In the top-level CMakeLists.txt maybe:
  
if(WIN32)
 set(DBUS_LIBEXECDIR)
else()
 set(DBUS_LIBEXECDIR ${KDE_INSTALL_FULL_LIBEXECDIR}/kf5/)
endif()
  
  ... and then use that everywhere needed.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Ralf Habacker]

habacker added a comment.


  In https://phabricator.kde.org/D9423#181442, @kfunk wrote:
  
  > And I agree. There shouldn't be a need to use two different input files. 
That's the whole point of `configure_file(...)`: the interpolation of values 
happens *inside* the input files.
  
  
  This would be possible by using in the service file something like this
  
  [D-BUS Service]
  Name=org.kde.kiod5
  Exec=@SOME_PREFIX@kiod5
  
  and in CMakeLists.txt
  
  if(WIN32)
  
set(SOME_PREFIX)
  
  else()
  
set(SOME_PREFIX ${KDE_INSTALL_FULL_LIBEXECDIR}/kf5/)
  
  endif()
  
  configure_file(org.kde.kiod5.service.in 
${CMAKE_CURRENT_BINARY_DIR}/org.kde.kiod5.service)
  
  Any problem with this ?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Kevin Funk]

kfunk requested changes to this revision.
kfunk added a comment.
This revision now requires changes to proceed.


  And I agree. There shouldn't be a need to use two different input files. 
That's the whole point of `configure_file(...)` the interpolation of values 
happens *inside* the input files.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley, kfunk
Cc: kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Kevin Funk]

kfunk added a comment.


  Could you please squash the commits and just keep this review for all the 
changes in KIO? Doesn't make sense to have different Phab Diffs for that (makes 
it harder to review, harder to revert in case, etc.).

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley
Cc: kfunk, broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Kai Uwe Broulik]

broulik added a comment.


  Is it possible to move that logic into the `.in` file instead of changing the 
cmake file? Just asking, I don't know cmake well.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D9423

To: habacker, dfaure, bcooksley
Cc: broulik, #frameworks

D9423: Fix 'Exec line in kiod service file must not have any path prefix on Windows'

[Ralf Habacker]

habacker created this revision.
habacker added reviewers: dfaure, bcooksley.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  This commit uses a similar approach as https://phabricator.kde.org/D9383
  BUG:382459
  FIXED-IN:5.42.0

TEST PLAN
  cross compiled

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D9423

AFFECTED FILES
  src/kiod/CMakeLists.txt
  src/kiod/org.kde.kiod5.service.win.in

To: habacker, dfaure, bcooksley
Cc: #frameworks