Did commit 4a543072 break cmake?

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Did commit 4a543072 break cmake?

Inkscape - Dev mailing list
Dear Inkscape developers,

I just updated to latest master und get the impression that commit
4a543072 broke the main CMakeList.txt file.

At least I get this error running CMake:

CMake Error at CMakeLists.txt:170 (add_custom_target):
  add_custom_target Wrong syntax.  Unknown type of argument.

and this commit changed this line.

See:

https://gitlab.com/inkscape/inkscape/commit/4a543072019e0bd4a0fcaaa3c2cef718e045edc6

Best regards,

Michael

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Did commit 4a543072 break cmake?

Eduard Braun
Hi Michael,

could you provide a full log (CMakeOutput.log / CMakeError.log)? From
the excerpt you posted I can't guess what's wrong...

As CI is working fine I suspect some kind of cmake incompatibility...

Regards,
Eduard


Am 20.07.2017 um 23:11 schrieb Michael Soegtrop via Inkscape-devel:

> Dear Inkscape developers,
>
> I just updated to latest master und get the impression that commit
> 4a543072 broke the main CMakeList.txt file.
>
> At least I get this error running CMake:
>
> CMake Error at CMakeLists.txt:170 (add_custom_target):
>    add_custom_target Wrong syntax.  Unknown type of argument.
>
> and this commit changed this line.
>
> See:
>
> https://gitlab.com/inkscape/inkscape/commit/4a543072019e0bd4a0fcaaa3c2cef718e045edc6
>
> Best regards,
>
> Michael
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Inkscape-devel mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/inkscape-devel


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Did commit 4a543072 break cmake?

Inkscape - Dev mailing list
Dear Eduard,

> could you provide a full log (CMakeOutput.log / CMakeError.log)? From
the excerpt you posted I can't guess what's wrong...

I figured it out - I think your change requires at least CMake 3.2, I
had 3.0.2 and the minimum requirement in the main CMake file is 2.8.2.
See my post on CMake versions.

Best regards,

Michael


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Did commit 4a543072 break cmake?

Eduard Braun
Am 22.07.2017 um 09:03 schrieb Michael Soegtrop:
> I figured it out - I think your change requires at least CMake 3.2, I
> had 3.0.2 and the minimum requirement in the main CMake file is 2.8.2.
> See my post on CMake versions.

Should be fixed in
https://gitlab.com/inkscape/inkscape/commit/d48909a51f4698bff7a1a172131c297aee15338f

The "specific syntax" you mentioned (next time please share so I don't
have to research it again ;-) ) seems to be the USES_TERMINAL option on
add_custom_target which should not be required in this case.

Dropping support for old cmake versions was not desired, I simply
overlooked that one.

@all: As CMake documentation does not mention when specific features
were introduced, does anybody know of a good document that summarized
and links syntax changes to cmake versions? The best way I could come up
with so far (short of browsing the source repository) is to switch
through the documentation versions, but that's obiously not very
convenient. (Or might there even be a compatibility checker available?)

Best regards,
Eduard

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Did commit 4a543072 break cmake?

Inkscape - Dev mailing list
Dear Eduard,

> Should be fixed in

thanks! At least one item on my list of Debian 8 incompatibilities solved.

> next time please share so I don't have to research it again ;-)

I didn't check what exactly the issue was - all I found it is that the
error happens in the line I mentioned in my initial post and that it
goes away with cmake 3.6.2.

> Or might there even be a compatibility checker available?

I would suggest to run CI tests with exactly the version of CMake which
is mentioned in the main CMake file as minimum required version. This
way such issues would show up during CI testing.

It is a good question what version this should be. I can confirm that up
to very recently it did work with 3.0.2. Not sure if it really works
with 2.8.2 as mentioned in the main CMake file.

Best regards,

Michael

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Did commit 4a543072 break cmake?

Eduard Braun
Am 22.07.2017 um 14:23 schrieb Michael Soegtrop:
> I didn't check what exactly the issue was - all I found it is that the
> error happens in the line I mentioned in my initial post and that it
> goes away with cmake 3.6.2.
Then I misinterpreted your earlier comment ("I figured it out -I think
your change requires at least CMake 3.2"). Sounded as if you had already
tracked down the specific issue. In that case you made a very good
prediction as it turned out this was exactly the version that would've
been required. ;-)

> I would suggest to run CI tests with exactly the version of CMake which
> is mentioned in the main CMake file as minimum required version. This
> way such issues would show up during CI testing.
That sounds like a good idea!
Unfortunately we can't easily do that in all cases (e.g. MSYS2 uses
rolling release and therefore will always uses the most recent version
of cmake - which is also not bad as it makes us aware of new issues early).
But maybe GitLab CI on Linux allows for this? I have no experience with
it, though, but I guess Mc or tedg can comment!
Also having CI use different distribution versions (as we had on
Launchpad) would help to avoid issues like the ones you mentioned in the
other thread. I think bryce wanted to look into setting up the ppa again
eventually.

> It is a good question what version this should be. I can confirm that up
> to very recently it did work with 3.0.2. Not sure if it really works
> with 2.8.2 as mentioned in the main CMake file.
Until recently it was 2.8.0 and I changed it to 2.8.2 because I added
something new (can't remember what). Now I think it should even be
2.8.11 because of the use of "string(TIMESTAMP ...)" - but there could
obviously already be code that needs a newer version...

Regards,
Eduard

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Loading...