bug/3135/build-issues #89

Συγχωνευμένα
SlavekB συγχώνευσε 2 υποβολές από bug/3135/build-issues σε master 4 έτη πριν
aneejit1 σχολίασε 4 έτη πριν
Συνεργάτης

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 αξιολόγησε 4 έτη πριν
SlavekB άφησε ένα σχόλιο
Ιδιοκτήτης

Here are some comments we should address.

Here are some comments we should address.
dnssd/CMakeLists.txt Παρωχημένο
LINK tdecore-shared ${AVAHI_TQT_LIBRARIES} ${AVAHI_CLIENT_LIBRARIES}
DEPENDENCIES tdeconfig_compiler
LINK tdecore-shared
LINK_PRIVATE ${AVAHI_TQT_LIBRARIES} ${AVAHI_CLIENT_LIBRARIES}
SlavekB σχολίασε 4 έτη πριν
Ιδιοκτήτης

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.
dnssd/CMakeLists.txt Παρωχημένο
DEPENDENCIES tdeconfig_compiler
LINK tdecore-shared
LINK_PRIVATE ${AVAHI_TQT_LIBRARIES} ${AVAHI_CLIENT_LIBRARIES}
DEPENDENCIES tdeconfig_compiler DCOP-shared
SlavekB σχολίασε 4 έτη πριν
Ιδιοκτήτης

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?
aneejit1 σχολίασε 4 έτη πριν
Συντάκτης
Συνεργάτης

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?
SlavekB σχολίασε 4 έτη πριν
Ιδιοκτήτης

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.
kdoctools/CMakeLists.txt Παρωχημένο
# 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 )
SlavekB σχολίασε 4 έτη πριν
Ιδιοκτήτης

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.
aneejit1 σχολίασε 4 έτη πριν
Συντάκτης
Συνεργάτης

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?
SlavekB σχολίασε 4 έτη πριν
Ιδιοκτήτης

Yes, that sounds like a good solution.

Yes, that sounds like a good solution.
kdoctools/ConfigureChecks.cmake Παρωχημένο
# update entities
message( STATUS "Updating ${ENTITIES_FILE}
message( STATUS "Updating ${ENTITIES_FILE_SRC}
SlavekB σχολίασε 4 έτη πριν
Ιδιοκτήτης

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.
kglib/CMakeLists.txt Παρωχημένο
VERSION 0.0.0
LINK ${TQT_LIBRARIES} ${GLIB2_LIBRARIES} ${GOBJECT2_LIBRARIES}
LINK ${TQT_LIBRARIES}
LINK_PRIVATE ${GLIB2_LIBRARIES} ${GOBJECT2_LIBRARIES}
SlavekB σχολίασε 4 έτη πριν
Ιδιοκτήτης

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.
tdecore/CMakeLists.txt Παρωχημένο
LINK_PRIVATE ltdlc-static ${KDESVGICONS} ${XCOMPOSITE_LIBRARIES}
${LIBIDN_LIBRARIES} ${LIBBFD_LIBRARIES} ${LIB_UTIL} ${GAMIN_LIBRARIES}
DESTINATION ${LIB_INSTALL_DIR}
DEPENDENCIES dcopidl2cpp
SlavekB σχολίασε 4 έτη πριν
Ιδιοκτήτης

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`.
aneejit1 σχολίασε 4 έτη πριν
Συντάκτης
Συνεργάτης

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.
SlavekB σχολίασε 4 έτη πριν
Ιδιοκτήτης

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.
SlavekB πρόσθεσε μια νέα εξάρτηση 4 έτη πριν
SlavekB σχολίασε 4 έτη πριν
Ιδιοκτήτης

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.
aneejit1 σχολίασε 4 έτη πριν
Συντάκτης
Συνεργάτης

OK, done that.

OK, done that.
SlavekB σχολίασε 4 έτη πριν
Ιδιοκτήτης

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?
aneejit1 σχολίασε 4 έτη πριν
Συντάκτης
Συνεργάτης

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.
SlavekB σχολίασε 4 έτη πριν
Ιδιοκτήτης

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 ενέκρινε αυτές τις αλλαγές 4 έτη πριν
SlavekB άφησε ένα σχόλιο
Ιδιοκτήτης

Great, it looks good.

Great, it looks good.
SlavekB έκλεισε αυτό το pull request 4 έτη πριν
SlavekB διέγραψε το κλάδο bug/3135/build-issues 4 έτη πριν
SlavekB το πρόσθεσε στο R14.0.9 release ορόσημο 4 έτη πριν
Το pull request έχει συγχωνευθεί ως 2c6417b8d6.
Συνδεθείτε για να συμμετάσχετε σε αυτή τη συνομιλία.
Δεν υπάρχουν εξεταστές
Χωρίς Ορόσημο
Χωρίς Αποδέκτη
2 Συμμετέχοντες
Ειδοποιήσεις
Ημερομηνία Παράδοσης

Δεν ορίστηκε ημερομηνία παράδοσης.

Εξαρτάται από
Αναφορά: TDE/tdelibs#89
Φόρτωση…
Δεν υπάρχει ακόμα περιεχόμενο.