Bug 2695

CMake Warning (dev) at tdeabc/vcardparser/CMakeLists.txt:93 (add_custom_target):
  Policy CMP0037 is not set: Target names should not be reserved and should
  match a validity pattern.  Run "cmake --help-policy CMP0037" for policy
  details.  Use the cmake_policy command to set the policy and suppress this
  warning.

  The target name "test" is reserved or not valid for certain CMake features,
  such as generator expressions, and may result in undefined behavior.
This warning is for project developers.  Use -Wno-dev to suppress it.


pfui - learned something once again

I just pulled last sources from R14.1.x (tde and packaging) on debian/stretch and made a new clean rebuild from scratch (not only tdelibs but all packages required). No building issues whatsoever.
Are you using some additional local patches?

Someone else might try to reproduce. Are you sure the other patches are in. In my original patch I skipped the test target. I was hoping Fat-Zer will do some work as promised earlier, so that we may work in structured way.

This patch might be inappropriate for this bug as it is about the testing.

Sorry, I get more organized in terms of TDE recently. I hope I'll get much better in future.

> I just pulled last sources from R14.1.x (tde and packaging) on debian/stretch 
> and made a new clean rebuild from scratch (not only tdelibs but all packages 
> required). No building issues whatsoever.

> Someone else might try to reproduce. Are you sure the other patches are in. In 
> my original patch I skipped the test target. I was hoping Fat-Zer will do some 
> work as promised earlier, so that we may work in structured way.

Hi Emanoil, as I said I built from the latest sources and no problems.

What I was thinking (since vcardparser is something you have done some work on the past months) is that you may be using local patches not yet pushed to GIT and those causes the FTBFS.

Could you help to verify this as follow:

1) pull latest R14.1 sources and packaging files
2) make a clean build, without any additional local patch you may be using
3) check whether FTBFS happens or not and let us know.
4) if there was no FTBFS in 3), put back your local patch step by step and identify which patch causes the FTBFS. Then the patch attached to this bug report should go together with the local patch that created the problem

Thanks

Hi,
I did and indeed it builds. In the build log I do not see executing the test target.

bash test.sh
Checking: ./tests/vcard1.vcf   OK
Checking: ./tests/vcard2.vcf   OK
Checking: ./tests/vcard3.vcf   OK
Checking: ./tests/vcard4.vcf   OK
Checking: ./tests/vcard6.vcf   OK
Checking: ./tests/vcard7.vcf   OK
Checking: ./tests/vcard8.vcf   OK
Checking: ./tests/vcard9.vcf   OK

With the last patch test target is executed.

regards

ah, you meant it failed to execute the testing, not that it FTBFS. Perhaps I misunderstood. 
I will test the patch and if good I will commit with your name. How about the CLA? Can I sign off the commit with your name?

Patch is good and ready to be pushed. I have just removed those hunks which did not add any functionality, just empty white spaces at the end of the lines.

As soon as you feedback about the CLA (or just confirm that the patch is the result of your own work [which I have no doubt at all] ), I will commit it.

Thanks for the good work!

Yes Michele. I had a look also I submitted originally without enabling the test target. After I enabled the test target per default it did not execute, because the dependencies were wrongly set.

You could change the target name from test to something else - perhaps testvcards, to skip the CMP0037 warning.

regards

> As soon as you feedback about the CLA (or just confirm that the patch is the 
> result of your own work [which I have no doubt at all] ), I will commit it.

What I meant is: what is the status of your CLA? See the "individual contributor" one.
https://www.trinitydesktop.org/cla/

Otherwise you just need to confirm (here is ok) the patch is the result of your own work (which I am sure it is :-) ), then I can commit the patch in your name

Hi thank you for explanation.
I have not signed CLA (yet). I confirm here that the patch is a result of my own work.
I will study the CLA an come back later.

Pushed in commit 13e0329 (R14.1) and 83681c9 (R14.0).
Thanks for the great work Emanoil!

There is a problem with patch:

/usr/bin/make -f tdeabc/vcardparser/CMakeFiles/test.dir/build.make tdeabc/vcardparser/CMakeFiles/test.dir/build
make[3]: Entering directory `/build/buildd/tdelibs-trinity-14.1.0~r1374/obj-arm-linux-gnueabi'
make[3]: *** No rule to make target `testing', needed by `tdeabc/vcardparser/CMakeFiles/test'.  Stop.
make[3]: Leaving directory `/build/buildd/tdelibs-trinity-14.1.0~r1374/obj-arm-linux-gnueabi'
make[2]: *** [tdeabc/vcardparser/CMakeFiles/test.dir/all] Error 2

See build
https://quickbuild.pearsoncomputing.net/~trinity/+archive/ubuntu/trinity-nightly-builds/+build/333734

Strange, it built ok yesterday when I tested.
I will test again tomorrow, today no more time (need to sleep)

(In reply to Michele Calgaro from comment #13)
> Strange, it built ok yesterday when I tested.
> I will test again tomorrow, today no more time (need to sleep)

It seems that the problem had manifested itself only on Debian 6 (Squeeze) and Ubuntu 10.04 (Lucid). I'll try to look at it later.

you have in the log
[100%] Built target testing

and next

Scanning dependencies of target test
make[3]: Leaving directory `/build/buildd/tdelibs-trinity-14.1.0~r1374/obj-arm-linux-gnueabi'
/usr/bin/make -f tdeabc/vcardparser/CMakeFiles/test.dir/build.make tdeabc/vcardparser/CMakeFiles/test.dir/build
make[3]: Entering directory `/build/buildd/tdelibs-trinity-14.1.0~r1374/obj-arm-linux-gnueabi'
make[3]: *** No rule to make target `testing', needed by `tdeabc/vcardparser/CMakeFiles/test'.  Stop.

I hope it helps

Created attachment 2704 [details]
Fix FTBFS on VCardParser tests

If I remember correctly, the names of targets in CMake should be unique in project - hence the names 'test' and 'testing' look very inappropriately. Additionally, creating a test environment does not need to be as a 'target', but may be as a single command. Because creating a test environment in a single command does not need another dependencies, is not called repeatedly == no need to test if symlinks already exists.

In the attached patch has changed the name of the target and simplified commands for creating a test environment. Tested on Stretch and Squeeze => both fine. It remains awkward that the test itself is carried out twice. However, it was also with the prior patch.

One more note: test.sh looks like it does not contain bashism's == instead of 'bash' could be called 'sh'.

Thank you Slavek.
I agree. You can read the discussion on the mailing list. There needs to be a test infrastructure throughout the CMake files. I am not that familiar with CMake on how to do it, but it should be consistent through all packages. Fat-Zer mentioned something about changing the packaging files.

Regarding the test.sh ... it shouldn't be there, but I had no idea how to write this in the cmake definition. The second reason was that tests were not working properly and I had to bypass them. I think I know now better.

I suggest we do it right at this level and take care of the global testing later.

Created attachment 2707 [details]
new patch

Slavek's patch looks good. I also tested the same patch replacing "bash" with "sh" and the result was good.
I have attached the modified patch.

Created attachment 2708 [details]
Fundamentally new patch :)

Script test.sh is gone.
Preparation of the test environment is gone.
...everything is cleanly directly in CMake :)

Please, test also on your builder.

Also the new patch looks good.

Pushed to GIT in hash 1571e617 (master) and aca39581 (r14.0.x)

yes indeed, thank you Slavek and you Michele as well