Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cmake][linux] Several fixes #9568

Merged
merged 11 commits into from Apr 16, 2016

Conversation

Projects
None yet
6 participants
@hudokkow
Copy link
Member

commented Apr 6, 2016

These are the low hanging ones...

Pending:

  • generate kodi-config.cmake / xbmc-config.cmake from *.in files and install

Problems:

  • folders with spaces in path aren't copied to install folder correctly (they are correct in build folder and properly quoted in cmake_install.cmake). See screenshot below.
  • lib/kodi/system/players/VideoPlayer/libdvd***-x86_64-linux.so are not installed and I can't find them under cmake build/ folder. Something wrong with the build?
  • a bunch of binaries are different of the autotools generated ones
  • still not able to start kodi in the install folder:

hudo@hudo:~/dev/tmp/c/bin$ ./kodi -p
appPath: /home/hudo/dev/tmp/d/lib/kodi
Unable to find path to Kodi data files!

BTW, why are we installing empty folders like share/kodi/addons/library.*?

virtualbox_2016-04-06_14-58-45

ping @fetzerch and @notspiff

system/peripherals.xml
system/playercorefactory.xml
system/X10-Lola-IRSSmap.xml

This comment has been minimized.

Copy link
@fetzerch

fetzerch Apr 6, 2016

Member

X10-Lola-IRSSmap.xml and IRSSmap.xml are used in project/cmake/installdata/windows/irss.txt. If we need them everywhere, could you drop them there?

This comment has been minimized.

Copy link
@hudokkow

hudokkow Apr 7, 2016

Author Member

I don't know if they are used at all under linux. I just want diff -qr autotools/ cmake/ | sort to return a nice nada. 😉

This comment has been minimized.

Copy link
@notspiff

notspiff Apr 7, 2016

Contributor

I didn't put them in windows only just for the heck of it. They are only
used on windows, that is, they are the same file, the lola one is an
example for a specific remote and wont be used by the installation as such.
This is keymaps for 'lirc for windows'.
7. apr. 2016 15:43 skrev "Cristiano A. Silva" notifications@github.com:

In project/cmake/installdata/common/common.txt
#9568 (comment):

system/peripherals.xml
system/playercorefactory.xml
+system/X10-Lola-IRSSmap.xml

I don't know if they are used at all under linux. I just want that diff
-qr autotools/ cmake/ | sort returns a nice nada. [image: 😉]


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/xbmc/xbmc/pull/9568/files/5b9d2d52b4166c3e2e00be21ebad70f596d9b4c6..d8f6977c16c01c2a53242814a9c7494e8e93eb99#r58872648

This comment has been minimized.

Copy link
@akva2

akva2 Apr 7, 2016

Contributor

system/python shall always be empty on linux so why install it. replicating old bugs is not an acceptable answer.

@fetzerch

This comment has been minimized.

Copy link
Member

commented Apr 6, 2016

Thanks, yes that's certainly an area where we have to clean up things a bit.
I didn't look much into the installation so far, but your changes look good to me!
If @notspiff likes, a second pair of eyes would certainly not hurt.

Empty folders shouldn't be installed, looks like a bug.
For the libraries, I'll need to check that. Does Kodi run from the build dir?

@hudokkow hudokkow force-pushed the hudokkow:cmake branch from 25c34e1 to 1fe637b Apr 7, 2016

@hudokkow

This comment has been minimized.

Copy link
Member Author

commented Apr 7, 2016

24bedf9 cures problems with spaces/parentheses/whatever in paths during install to target folder.
Is it correct or did I got lucky and doing something completely wrong?

btw, it also fixes

-- Prefix: /home/hudo/dev/tmp/c
-- Libdir: /home/hudo/dev/tmp/c/lib
-- Bindir: /home/hudo/dev/tmp/c/bin
-- Includedir: /home/hudo/dev/tmp/c/include
-- Datarootdir: /home/hudo/dev/tmp/c/share
-- Datadir: ${datarootdir}

and now shows

-- Prefix: /home/hudo/dev/tmp/c
-- Libdir: /home/hudo/dev/tmp/c/lib
-- Bindir: /home/hudo/dev/tmp/c/bin
-- Includedir: /home/hudo/dev/tmp/c/include
-- Datarootdir: /home/hudo/dev/tmp/c/share
-- Datadir: /home/hudo/dev/tmp/c/share

@akva2

This comment has been minimized.

Copy link
Contributor

commented Apr 7, 2016

The folders arent empty, there is a hidden dummy file in there (@AlwinEsch can explain em).

@notspiff

This comment has been minimized.

Copy link
Contributor

commented Apr 7, 2016

this should be correct. i have lost the history of the code due to a
gazillion rebases, but i'm sure @wsnipex, who wrote that code, had a reason
for the \ at some point. there's no reason now.

@hudokkow

This comment has been minimized.

Copy link
Member Author

commented Apr 7, 2016

Found out why it doesn't run inside the target install folder. bin/kodi is wrong:

--- kodi_a 2016-04-07 13:41:08.660258646 +0100
+++ kodi_c 2016-04-07 14:57:17.251425396 +0100
@@ -21,10 +21,10 @@
APP=Kodi
bin_name=kodi
SAVED_ARGS="$@"
-prefix="/home/hudo/dev/tmp/a"
-exec_prefix="/home/hudo/dev/tmp/a"
-datarootdir="${prefix}/share"
-LIBDIR="${exec_prefix}/lib"
+prefix="/home/hudo/dev/tmp/c"
+exec_prefix="/home/hudo/dev/tmp/c"
+datarootdir="/home/hudo/dev/tmp/c/share"
+LIBDIR="/home/hudo/dev/tmp/c/lib"
CRASHLOG_DIR=${CRASHLOG_DIR:-$HOME}
USERDATA_DIR="${HOME}/.${bin_name}"

Now to fix the file.

@notspiff, yeah noticed that 20m ago. .gitignore files

EDIT: Disregard. My error. Back to the drawing board.

@akva2

This comment has been minimized.

Copy link
Contributor

commented Apr 7, 2016

and sorry if i come off as unnecessarily snarky, i'm under a lot of pressure at work and this is procrastination on a high level. not fair to take it out on innocents. i know but bare with me ;)

@hudokkow

This comment has been minimized.

Copy link
Member Author

commented Apr 7, 2016

No problem Arne. 😉 Don't forget to have some 🍻

@wsnipex

This comment has been minimized.

Copy link
Member

commented Apr 9, 2016

For ref, datarootdir was supposed to be a shell var and has worked as is back then. Quite a lot has changed since then, so its possible something broke, but its not obvious to me what

@hudokkow hudokkow force-pushed the hudokkow:cmake branch 2 times, most recently from 8075635 to e4fd1f9 Apr 9, 2016

@hudokkow

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2016

@popcornmix, you might want to test this with rpi. In particular dfa8149 solves the run from install dir issues you mentioned in #9340 (comment) and that I was also experiencing in linux.
It only cost me 4 or 5 days to nail that down... and I must have looked at it 300+ times.

@fetzerch, other OSes pathsetup.cmake also use xbmc/. Is this by design? I assume kodi/ is the main dir and we just symlink xbmc to it.

@hudokkow hudokkow force-pushed the hudokkow:cmake branch from dfa8149 to 6073a28 Apr 11, 2016

@fetzerch

This comment has been minimized.

Copy link
Member

commented Apr 12, 2016

Yes, good catch. Could you add a fix for the other platforms as well?

Do you have more fixes in the queue, otherwise we could merge this soon.
I'd also prefer to drop 2b0a33c however.

@hudokkow hudokkow force-pushed the hudokkow:cmake branch 2 times, most recently from 13764f4 to 2af1c1a Apr 12, 2016

@hudokkow

This comment has been minimized.

Copy link
Member Author

commented Apr 12, 2016

Dropped 2b0a33c and squashed what seemed logic.
I have other fixes on the way but I probably can't test them before the weekend so might as well get this in.
This was only tested on Linux. Hopefully other platforms are OK.

@hudokkow hudokkow force-pushed the hudokkow:cmake branch 2 times, most recently from b456499 to 9f8c4e1 Apr 13, 2016

@hudokkow

This comment has been minimized.

Copy link
Member Author

commented Apr 14, 2016

Added two more commits.

Fun fact: autotools and cmake don't like each other. If you try to build with cmake after you built with autotools, its gonna fail.

I thought I was being clever by issuing export CCACHE_DIR=~/.ccache-cmake at the start of cmake build but I continued to observe problems with the build.
Took me a few hours to finally understand what was going on and git clean the tree.

Its probably because the flags are different, config.h isn't being generated, etc.

@notspiff

This comment has been minimized.

Copy link
Contributor

commented Apr 14, 2016

it's because autotools does everything in-tree. it generates the buildsystems for stuff which are generated out-of-tree with cmake. with those in place, you cannot do an out-of-tree afterwards.

@hudokkow

This comment has been minimized.

Copy link
Member Author

commented Apr 14, 2016

Yeah, not fun.
Anyways, lesson learned.

@hudokkow hudokkow force-pushed the hudokkow:cmake branch from 5bc27ec to 5ac3c5f Apr 15, 2016

@hudokkow hudokkow force-pushed the hudokkow:cmake branch 2 times, most recently from 98f80ac to f7805c5 Apr 15, 2016

@hudokkow hudokkow force-pushed the hudokkow:cmake branch from f7805c5 to e399999 Apr 15, 2016

@fetzerch

This comment has been minimized.

Copy link
Member

commented Apr 16, 2016

Changes look good to me, so feel free to merge.

@hudokkow

This comment has been minimized.

Copy link
Member Author

commented Apr 16, 2016

Thanks! Merging now.
Just so we don't duplicate work, my todo list (so far) includes:

A couple of questions:

  • why are cpluff configure files being generated in-tree? It's the only dep tainting the tree
  • why are we building libdvd*** static? autotools builds them shared
  • what platforms already have jenkins cmake build support?

@hudokkow hudokkow merged commit 2bb2b5e into xbmc:master Apr 16, 2016

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.