bug/3135/build-issues #89

Merged
SlavekB merged 2 commits from bug/3135/build-issues into master 4 years ago
Collaborator

Changes to:

  • dnssd/CMakeLists.txt to include include/link directories for avahi-tqt

  • kdoctools/CMakeLists.txt and kdoctools/ConfigureChecks.cmake to build and install customization/entities/general.entities using the binary directory

  • kglib/CMakeLists.txt to add missing glib link directories

  • tdecore/CMakeLists.txt to add dependency for dcopidl2cpp to ensure it is built before it is used

Changes to: - dnssd/CMakeLists.txt to include include/link directories for avahi-tqt - kdoctools/CMakeLists.txt and kdoctools/ConfigureChecks.cmake to build and install customization/entities/general.entities using the binary directory - kglib/CMakeLists.txt to add missing glib link directories - tdecore/CMakeLists.txt to add dependency for dcopidl2cpp to ensure it is built before it is used
SlavekB reviewed 4 years ago
SlavekB left a comment
Owner

Here are some comments we should address.

Here are some comments we should address.
LINK tdecore-shared ${AVAHI_TQT_LIBRARIES} ${AVAHI_CLIENT_LIBRARIES}
DEPENDENCIES tdeconfig_compiler
LINK tdecore-shared
LINK_PRIVATE ${AVAHI_TQT_LIBRARIES} ${AVAHI_CLIENT_LIBRARIES}
Owner

Ok, this seems to be safe – I did a test of building tdegames where tdednssd-shared is used and it builds without problems.

Ok, this seems to be safe – I did a test of building tdegames where tdednssd-shared is used and it builds without problems.
DEPENDENCIES tdeconfig_compiler
LINK tdecore-shared
LINK_PRIVATE ${AVAHI_TQT_LIBRARIES} ${AVAHI_CLIENT_LIBRARIES}
DEPENDENCIES tdeconfig_compiler DCOP-shared
Owner

tdednssd-shared depends on tdecore-shared, where tdecore-shared already depends on DCOP-shared – see tdecore/CMakeLists.txt.

Why do you see a reason to add dependency for DCOP-shared here again?

tdednssd-shared depends on tdecore-shared, where tdecore-shared already depends on DCOP-shared – see [tdecore/CMakeLists.txt](src/branch/master/tdecore/CMakeLists.txt#L145). Why do you see a reason to add dependency for DCOP-shared here again?
Poster
Collaborator

I was running cmake multi-process and getting problems one of which was attempts to link libDCOP before it was ready. I thought I'd removed it when I went back to single process. Given that I've got multiple commits, how do I get rid of it?

I was running cmake multi-process and getting problems one of which was attempts to link libDCOP before it was ready. I thought I'd removed it when I went back to single process. Given that I've got multiple commits, how do I get rid of it?
Owner

It will now be possible to remove not only the DCOP-shared dependency, but also tdeconfig_compiler dependency! See #90.

To do change in a specific commit you can use:

  1. Use git rebase -i master
  2. An editor will open in which you can specify which commit you want to fix => change pick to edit.
  3. Close the editor and git will move to the commit you want to fix.
  4. Make the required changes and use git commit with the --amend argument to modify the existing commit.
  5. Use git rebase --continue to rebase other commits.
  6. Use git push origin HEAD -f to force a push to the new branch state.
It will now be possible to remove not only the DCOP-shared dependency, but also tdeconfig_compiler dependency! See #90. To do change in a specific commit you can use: 1. Use `git rebase -i master` 2. An editor will open in which you can specify which commit you want to fix => change `pick` to `edit`. 3. Close the editor and git will move to the commit you want to fix. 4. Make the required changes and use `git commit` with the `--amend` argument to modify the existing commit. 5. Use `git rebase --continue` to rebase other commits. 6. Use `git push origin HEAD -f` to force a push to the new branch state.
# done ;\
# done )
install( DIRECTORY customization docbook DESTINATION ${DATA_INSTALL_DIR}/ksgmltools2 PATTERN ".svn" EXCLUDE )
install( FILES ${CMAKE_CURRENT_BINARY_DIR}/customization/entities/general.entities DESTINATION ${DATA_INSTALL_DIR}/ksgmltools2/customization/entities )
Owner

If the general.entities file is to be installed from a binary folder, it should be excluded from installing other files from the source folder.

If the `general.entities` file is to be installed from a binary folder, it should be excluded from installing other files from the source folder.
Poster
Collaborator

I suppose there's no guarantee as to the order the commands are run in. I'm thinking adding

REGEX "/entities/general.entities$" EXCLUDE

after the existing "EXCLUDE" would do the job. There's an "obsolete/general.entities" that still needs to be copied, so the above would not exclude it. Would that be OK?

I suppose there's no guarantee as to the order the commands are run in. I'm thinking adding REGEX "/entities/general.entities$" EXCLUDE after the existing "EXCLUDE" would do the job. There's an "obsolete/general.entities" that still needs to be copied, so the above would not exclude it. Would that be OK?
Owner

Yes, that sounds like a good solution.

Yes, that sounds like a good solution.
# update entities
message( STATUS "Updating ${ENTITIES_FILE}
message( STATUS "Updating ${ENTITIES_FILE_SRC}
Owner

It seems that updating the data to the binary will only work because the TDE_VERSION_STRING, TDE_RELEASE_DATE, and TDE_RELEASE_COPYRIGHT entities are not currently used in the manuals generated within tdelibs. Here's the question of whether to rely on it?

You can see that generating the tdespell manual uses docbook files from the source folder – because of that it gets general.entitities with no items added during build.

We can either leave it unchanged with the risk that future documentation could fail due to this, or we would have to copy the entire contents of the docbook directory to a binary folder.

It seems that updating the data to the binary will only work because the `TDE_VERSION_STRING`, `TDE_RELEASE_DATE`, and `TDE_RELEASE_COPYRIGHT` entities are not currently used in the manuals generated within tdelibs. Here's the question of whether to rely on it? You can see that generating the [tdespell manual](src/branch/master/doc/tdespell/CMakeLists.txt#L12) uses docbook files from the source folder – because of that it gets `general.entitities` with no items added during build. We can either leave it unchanged with the risk that future documentation could fail due to this, or we would have to copy the entire contents of the docbook directory to a binary folder.
VERSION 0.0.0
LINK ${TQT_LIBRARIES} ${GLIB2_LIBRARIES} ${GOBJECT2_LIBRARIES}
LINK ${TQT_LIBRARIES}
LINK_PRIVATE ${GLIB2_LIBRARIES} ${GOBJECT2_LIBRARIES}
Owner

Ok, this seems to be safe, because kglibs-shared is not currently used anywhere.

Ok, this seems to be safe, because kglibs-shared is not currently used anywhere.
LINK_PRIVATE ltdlc-static ${KDESVGICONS} ${XCOMPOSITE_LIBRARIES}
${LIBIDN_LIBRARIES} ${LIBBFD_LIBRARIES} ${LIB_UTIL} ${GAMIN_LIBRARIES}
DESTINATION ${LIB_INSTALL_DIR}
DEPENDENCIES dcopidl2cpp
Owner

Ok, it seems that the idea of setting the dependence on the level of the common cmake module does not work as intended. I'm trying to find a solution at the level of a common cmake module that could work as intended – so that there is no need for an explicit dependency specification.

I will try to solve the same one also for the dependency on tdeconfig_compiler.

Ok, it seems that the idea of setting the dependence on the level of the [common cmake module](../tde-common-cmake/src/branch/master/modules/TDEMacros.cmake#L554) does not work as intended. I'm trying to find a solution at the level of a common cmake module that could work as intended – so that there is no need for an explicit dependency specification. I will try to solve the same one also for the dependency on `tdeconfig_compiler`.
Poster
Collaborator

I have come across problems building tdelibs multi-process, but this one cropped up running single-process.

I have come across problems building tdelibs multi-process, but this one cropped up running single-process.
Owner

Yes, I did a single-process build test and I got the same problem. Thanks to change #90 there is no need to add an explicit dependency here, as well as dependencies that were used in the common TDE CMake module can be removed! That's a better solution than I hoped.

Yes, I did a single-process build test and I got the same problem. Thanks to change #90 there is no need to add an explicit dependency here, as well as dependencies that were used in the common TDE CMake module can be removed! That's a better solution than I hoped.
Owner

You can rebase your PR to the current master:

git checkout bug/3135/build-issues
git rebase master

Because the changes between the master and your branch are consistent, there should be an automatic merge without having to manually resolve conflicts. Then you can push using force.

You can rebase your PR to the current master: ``` git checkout bug/3135/build-issues git rebase master ``` Because the changes between the master and your branch are consistent, there should be an automatic merge without having to manually resolve conflicts. Then you can push using force.
Poster
Collaborator

OK, done that.

OK, done that.
Owner

Now I noticed – in the original design, for tdednssd-shared you suggested moving {AVAHI_TQT_LIBRARIES} and {AVAHI_CLIENT_LIBRARIES} to LINK_PRIVATE. Now it's gone. Was it discarded on purpose?

Now I noticed – in the original design, for tdednssd-shared you suggested moving ${AVAHI_TQT_LIBRARIES} and ${AVAHI_CLIENT_LIBRARIES} to LINK_PRIVATE. Now it's gone. Was it discarded on purpose?
Poster
Collaborator

I moved them to LINK_PRIVATE more as a precaution, but I think the only other one that has a problem with not finding it is tdebase. I was having some problems trying to get things to refresh properly and I think I managed to lose that particular alteration along the way. I can put it back if you think it's a good idea.

I moved them to LINK_PRIVATE more as a precaution, but I think the only other one that has a problem with not finding it is tdebase. I was having some problems trying to get things to refresh properly and I think I managed to lose that particular alteration along the way. I can put it back if you think it's a good idea.
Owner

I moved them to LINK_PRIVATE more as a precaution, but I think the only other one that has a problem with not finding it is tdebase. I was having some problems trying to get things to refresh properly and I think I managed to lose that particular alteration along the way. I can put it back if you think it's a good idea.

Moving libraries to LINK_PRIVATE for tdednssd-shared seemed like a good idea. So yes, you can put it there.

> I moved them to LINK_PRIVATE more as a precaution, but I think the only other one that has a problem with not finding it is tdebase. I was having some problems trying to get things to refresh properly and I think I managed to lose that particular alteration along the way. I can put it back if you think it's a good idea. Moving libraries to LINK_PRIVATE for tdednssd-shared seemed like a good idea. So yes, you can put it there.
SlavekB approved these changes 4 years ago
SlavekB left a comment
Owner

Great, it looks good.

Great, it looks good.
SlavekB closed this pull request 4 years ago
SlavekB deleted branch bug/3135/build-issues 4 years ago
SlavekB added this to the R14.0.9 release milestone 4 years ago
The pull request has been merged as 2c6417b8d6.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Reference: TDE/tdelibs#89
Loading…
There is no content yet.