Skip to content

Commit

Permalink
changed: install python files for debian in dist-packages and use sit…
Browse files Browse the repository at this point in the history 
…e-packages for others
  • Loading branch information
@opdenkamp
opdenkamp committed Jan 28, 2017
1 parent dd1af24 commit 0a97062
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
2 changes: 1 addition & 1 deletion debian/python-libcec.install
View file
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
usr/lib/python*/site-packages/cec/*
usr/lib/python*/dist-packages/cec/*
usr/bin/pyCecClient
9 changes: 7 additions & 2 deletions src/libcec/cmake/CheckPlatformSupport.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,15 @@ else()
DESTINATION python/cec
RENAME __init__.py)
else()
if(EXISTS "/etc/lsb-release")
SET(PYTHON_PKG_DIR "dist-packages")
else()
SET(PYTHON_PKG_DIR "site-packages")
endif()
install(TARGETS ${SWIG_MODULE_cec_REAL_NAME}
DESTINATION lib/python${PYTHON_VERSION}/site-packages/cec)
DESTINATION lib/python${PYTHON_VERSION}/${PYTHON_PKG_DIR}/cec)
install(FILES ${CMAKE_BINARY_DIR}/src/libcec/cec.py
DESTINATION lib/python${PYTHON_VERSION}/site-packages/cec
DESTINATION lib/python${PYTHON_VERSION}/${PYTHON_PKG_DIR}/cec
RENAME __init__.py)
endif()
endif()
Expand Down

11 comments on commit 0a97062

@mkreisl
Copy link

@mkreisl mkreisl commented on 0a97062 Mar 3, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is IMO stupid commit:
In file debian/python-libcec.install you are using hardcoded usr/lib/python/dist-packages/cec/** and in file src/libcec/cmake/CheckPlatformSupport.cmake you're setting path depending of /etc/lsb-release exists or not.
That never works for all situations.
Btw, apt-file tells me /etc/lsb-release is in package debian-edu-config, I would assume nobody has installed that package

@opdenkamp
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

dpkg -S /etc/lsb-release
base-files: /etc/lsb-release

I would assume that every Debian based distro installed has this, and it seems to be working well. Are you sure that's it's not a stupid comment?

@mkreisl
Copy link

@mkreisl mkreisl commented on 0a97062 Mar 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, that's strange. I do not have that file installed, checked 4 debian installations

@mkreisl
Copy link

@mkreisl mkreisl commented on 0a97062 Mar 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dpkg -L base-files | grep lsb-release shows NOTHING

@opdenkamp
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://packages.debian.org/sid/lsb-release

While it is intended for use by LSB packages, this command may also be useful for programmatically distinguishing between a pure Debian installation and derived distributions.

It's even advertised for this purpose in Debian's package db?

@opdenkamp
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that Debian has stuck it in a separate package, while Ubuntu stuck it in it's base package. Do you have some other file to distinguish between Debian(based) and something else? The dist-packages directory is something that's not standard, but something that Debian package maintainers chose to use.

@mkreisl
Copy link

@mkreisl mkreisl commented on 0a97062 Mar 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but it's lsb_release and not /etc/lsb-release

@mkreisl
Copy link

@mkreisl mkreisl commented on 0a97062 Mar 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use lsb_release -i?

lsb_release -i
Distributor ID: Debian

@opdenkamp
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that installed by default on plain Debian too? Then it would be an easy change.

@mkreisl
Copy link

@mkreisl mkreisl commented on 0a97062 Mar 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better solution:
check content of /etc/os-release
This file is part of base-files package

cat /etc/os-release 
PRETTY_NAME="Debian GNU/Linux 8 (jessie)"
NAME="Debian GNU/Linux"
VERSION_ID="8"
VERSION="8 (jessie)"
ID=debian
HOME_URL="http://www.debian.org/"
SUPPORT_URL="http://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"

@mkreisl
Copy link

@mkreisl mkreisl commented on 0a97062 Mar 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not alone 😄
Issue #314

Please sign in to comment.