Fix MODL_Coclustering double size in windows #97
Conversation
folmos-at-orange
force-pushed
the
79-khiops-cc-size-is-twice-as-it-was-in-1011-in-windows
branch
from
51f447c to
05397ae
Compare
There was a problem hiding this comment.
J'ai vérifié sur ma branche: ça corrige bien le problème ;)
Question
- pourquoi continuer à se référer à MPI_SUFFIX si on ne compile pas cet exe avec MPI (cf. genum)?
Suggestion
- si tu change les commentaires, il faudrait homogénéiser avec ceux des autres CMakeLists.txt (en particulier celui de MODL)
Good question. This impacts the installation let's wait for @bruno-at-orange to weigh in.
Ok |
There was a problem hiding this comment.
Je ne suis pas d'accord avec la modification des commentaires. Il faut que tu arretes de te focaliser sur les target. TOUT est target dans cmake. Par exemple pour, add_binary(TOTO) et add_executable(TITI) dans le commentaire, on ne va pas mettre qu'on ajoute la target TOTO et la target TOTI mais bien l'executable TOTO et la bibliothèque TITI car c'est ce qu'oin souhaite faire. Et que cmake passe par des targets, c'est secondaire et ça n'apporte absolument rien à la comprehension du fichier CMakeList qu'on est en train de commenter.
Ici en l'occurence on parle du binaire MODL_Coclustering ou du nom de ce binaire. D'ailleurs pour fedora on n'a pas du tout besoin de renommer la target mais bien le nom du binaire !!!
Mon commentaire n'était peut-être pas précis car je faisis un abus de le langage, en effet il ne faut pas renommer le binaire mais le nom du binaire, soit ! On aurait donc on fedora, binaries names built with mpi must be suffixed by _mpich. Je pense qu'il est important de commencer le commentairte par on fedora car ça ne concerne que fedora...
There was a problem hiding this comment.
Ok. I will be more precise for each target type (binary or library). I would keep separated the code for each of them to avoid bugs.
There was a problem hiding this comment.
It was also a formatting error of cmake-format I didn't mean to put On Fedora after the title of the section.
folmos-at-orange
force-pushed
the
79-khiops-cc-size-is-twice-as-it-was-in-1011-in-windows
branch
3 times, most recently
from
dd98f5e to
d96ccb7
Compare
folmos-at-orange
force-pushed
the
79-khiops-cc-size-is-twice-as-it-was-in-1011-in-windows
branch
from
d96ccb7 to
10d575a
Compare
The MODL_Coclustering.exe executable in windows had doubled in size because the MSVC link option `/INCREMENTAL` was activated. This wasn't the case for the original build with VS projects.
folmos-at-orange
force-pushed
the
79-khiops-cc-size-is-twice-as-it-was-in-1011-in-windows
branch
from
10d575a to
ac429b9
Compare
folmos-at-orange
dismissed
bruno-at-orange’s stale review
"Dismissed" just to merge
folmos-at-orange
deleted the
79-khiops-cc-size-is-twice-as-it-was-in-1011-in-windows
branch
The MODL_Coclustering.exe executable in windows had doubled in size because the MSVC link option
/INCREMENTALwas activated.This wasn't the case for the original build with VS projects.