search

jerryscript-project / iotjs

Platform for Internet of Things with JavaScript http://www.iotjs.net
Other
2.59k stars 439 forks source link

build: Install to system's path (prefixed by destdir) #1923

Closed rzr closed 3 years ago

rzr commented 5 years ago

This was introduced first in #1134 and then droped in #1848

This will be helpful for packaging, or example on Tizen install using RPM macro: "%make_install" of Debian's debhelper.

Work along CMAKE_INSTALL_PREFIX, or/and can be overloaded individualy to alternate locations.

Forwarded: https://github.com/Samsung/iotjs/pull/ Relate-to: https://github.com/Samsung/iotjs/pull/1134 Change-Id: Ib969a1d5c2e7bc402b455377fc57a94654aa74dc IoT.js-DCO-1.0-Signed-off-by: Philippe Coval philippe.coval@osg.samsung.com

daeyeon commented 5 years ago
  1. The install dir of lib on the else branch is different.
  2. In my second thought, using constant normal names (e.g: lib and bin) along with CMAKE_INSTALL_PREFIX looks enough.
  3. The paths for lib and bin are editable but not for include.
  4. INSTALL_PREFIX can be removed if CMAKE_INSTALL_PREFIX is guided.
rzr commented 3 years ago

Please first merge: https://github.com/jerryscript-project/iotjs/pull/1948

rzr commented 3 years ago

unsure this failure is related to this change: https://travis-ci.org/github/jerryscript-project/iotjs/jobs/736333817

rzr commented 3 years ago

This change is released in: https://tracker.debian.org/news/1184873/accepted-iotjs-10715-1-source-into-unstable/

akosthekiss commented 3 years ago

OK, I just realized that the second commit may not belong to this here at all but could have infiltrated this PR from #1948 . That's not good either. This PR should contain the relevant commit only.

LaszloLango commented 3 years ago

@rzr #1948 is landed, please rebase this PR.

P.S.: sorry about the slow response times, this repository is not very active these days. :(

rzr commented 3 years ago

Hi, I've rebased it if It's merged i will rebase debian version on this base.

Next it will be helpful to update jerryscript with this additional change in: https://github.com/jerryscript-project/jerryscript/pull/4667

rzr commented 3 years ago

Thanks for feedback , anyone available to review and merge this one and then I'll update the debian package

Cc: @akosthekiss , @zherczeg , @daeyeon , @galpeter, @yadd ...

zherczeg commented 3 years ago

Note: there is a change request, until that is resolved, this cannot be landed by github

rzr commented 3 years ago

OK, I just realized that the second commit may not belong to this here at all but could have infiltrated this PR from #1948 . That's not good either. This PR should contain the relevant commit only.

yes that's why I suggested to review the other one 1st now it's merged and the relevant commit is right base...

Please can anyone ping @akosthekiss to unblock this ?

rzr commented 3 years ago

I guess this can be now merged thx