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

Covered Resolving Missing *.so Library Error #2777

Merged

Conversation

oleksandr-shabelnyk
Copy link
Contributor

@markus2330 please review my PR

Basics

Check relevant points but please do not remove entries.
Do not describe the purpose of this PR in the PR description but:

  • Short descriptions should be in the release notes (added as entry in
    doc/news/_preparation_next_release.md which contains _(my name)_)
    Please always add something to the the release notes.
  • Longer descriptions should be in documentation or in design decisions.
  • Describe details of how you changed the code in commit messages
    (first line should have module: short statement syntax)
  • References to issues, e.g. close #X, should be in the commit messages.

Checklist

Check relevant points but please do not remove entries.
For docu fixes, spell checking, and similar none of these points below
need to be checked.

  • I added unit tests
  • I ran all tests locally and everything went fine
  • affected documentation is fixed
  • I added code comments, logging, and assertions (see Coding Guidelines)
  • meta data is updated (e.g. README.md of plugins and METADATA.ini)

Review

Reviewers will usually check the following:

Labels

  • Add the "work in progress" label if you do not want the PR to be reviewed yet.
  • Add the "ready to merge" label if the build server is happy and also you
    say that everything is ready to be merged.

@oleksandr-shabelnyk oleksandr-shabelnyk changed the title Covered Resolving Missing \*.so Library Error Covered Resolving Missing *.so Library Error Jun 12, 2019
@oleksandr-shabelnyk
Copy link
Contributor Author

The Travis CI build was timed out, although the tests seem to be OK. Any idea why?

Copy link
Contributor

@markus2330 markus2330 left a comment

Choose a reason for hiding this comment

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

Thank you! Some content is a bit misplaced.

doc/COMPILE.md Outdated
@@ -712,6 +712,32 @@ For reference, you can look into the
and the
[CMake in OpenWRT](https://github.com/openwrt/openwrt/blob/master/include/cmake.mk).

### Resolving Missing \*.so Library Error
Copy link
Contributor

Choose a reason for hiding this comment

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

What you write here is only about run-time problems, it is unrelated to compiling (and the recommended solution of how to deal with .so is already in this document a bit above (about RPATH).

Maybe best if you put a condensed version of this directly to contributing-clion.md because it is not enough for its own tutorial and I would actually not recommend to do it this way, except if the IDE does not support to run scripts/run_dev_env

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, you said it yourself I should put in into compile.md. Sure, it makes more sense to have in contributing-clion.md.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, for the confusion. I pointed you to COMPILE.md because there is already a description of the solution to your problem. The "library" not found is a topic that spreads from compiling, installing to run-time. Your solution is run-time only, though.

doc/COMPILE.md Outdated
Run this script to set environmental variables for development purpose:

```sh
. /home/osboxes/TU/libelektra/scripts/run_dev_env
Copy link
Contributor

Choose a reason for hiding this comment

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

This is specific to your config, it will not help someone else.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I will remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

When you write: "Let us assume that the source dir is in..., then..." it would be okay.

@sanssecours
Copy link
Member

The Travis CI build was timed out, although the tests seem to be OK. Any idea why?

Unfortunately the hardware Travis uses for macOS builds is quite slow. Since Travis has a timeout limit of 50 minutes for public repositories, this means sometimes a build job will fail, just because of this limitation 😢. The usual quick, but certainly no ideal fix for that problem, is to just rerun the build job that hit the timeout limit.

@alexsaber
Copy link
Contributor

@markus2330 you requested changes but did not say which ones. Can you please list them?

Copy link
Contributor

@markus2330 markus2330 left a comment

Choose a reason for hiding this comment

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

Thank you, nearly finished.

@@ -543,6 +543,7 @@ mounted, use `kdb gen -F <plugin>:<file> elektra <parentKey> <outputName>` to lo
- Added a section on `elektraPluginGetGlobalKeySet` in the plugin tutorial. _(Vid Leskovar)_
- Added a step-by-step [tutorial](../tutorials/run_all_tests_with_docker.md) for running all tests with Docker. _(Oleksandr Shabelnyk)_
- Added a step-by-step [tutorial](../tutorials/run_reformatting_script_with_docker.md) for running reformatting scripts with Docker. _(Oleksandr Shabelnyk)_
- Covered Resolving Missing \*.so Library Error in [tutorial](../tutorials/contributing-clion.md) and [tutorial](../COMPILE.md). _(Oleksandr Shabelnyk)_
Copy link
Contributor

Choose a reason for hiding this comment

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

Outdated information (COMPILE.md is not modified).

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you. I forgot to remove it.

LD_LIBRARY_PATH="/path/you/want1:/path/you/want/2"
```

Please keep in mind it sets the variable only in the currently opened shell window/session. To make it persistent please search online how to do it for you Linux distribution.
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you can refer to COMPILE.md (it contains instructions of how to fix the problem permanently.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

If you want to run built `kdb` outside of CLion, you can set the LD_LIBRARY_PATH variable like this in command line:

```sh
LD_LIBRARY_PATH="/path/you/want1:/path/you/want/2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Outside of CLion, please only recommend the script scripts/run_dev_env. The only reason to make it differently in CLion is that CLion currently does not support executing scripts for the environment. (Please also write that).

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

@markus2330
Copy link
Contributor

@markus2330 you requested changes but did not say which ones. Can you please list them?

I do not understand the question. I wrote two reviews and in the review comments it says what should be changed.

@alexsaber
Copy link
Contributor

@markus2330 you requested changes but did not say which ones. Can you please list them?

I do not understand the question. I wrote two reviews and in the review comments it says what should be changed.

When I left this comment I did not see your comments. I am sorry for the confusion.

Copy link
Contributor

@markus2330 markus2330 left a comment

Choose a reason for hiding this comment

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

Still not correct.

If you want to run built `kdb` outside of CLion, the recommended way is to run this script:

```sh
./scripts/run_dev_env
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to source this (. (space!) or source ), otherwise it does not work.

And you forgot to say that you need to be in the build dir.

See the example in the issue where you had this problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am so sorry. I fixed it.

./scripts/run_dev_env
```

If it does not work, there is another way around, which you should use only as the last resort. Set the LD_LIBRARY_PATH variable like this in command line:
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not recommend this. It is better if you describe how to source a script with the . operator. (You can also add this to doc/TESTING.md).

Copy link
Contributor

Choose a reason for hiding this comment

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

I will remove it. I think it is better.

LD_LIBRARY_PATH="/path/you/want1:/path/you/want/2"
```

Please keep in mind it sets the variable only in the currently opened shell window/session.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also correct for the scripts/run_dev_env but:

Suggested change
Please keep in mind it sets the variable only in the currently opened shell window/session.
Please keep in mind it sets the variables only in the currently opened shell window/session.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@alexsaber
Copy link
Contributor

@markus2330 thank you!
I changed everything. Is it good now?

If you want to run built `kdb` outside of CLion, the recommended way is to run this script from your build directory:

```sh
. /scripts/run_dev_env
Copy link
Contributor

Choose a reason for hiding this comment

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

Please try out what you write: here you assume scripts to be in root of the whole filesystem but it is in root of Elektra's source directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed it

@markus2330 markus2330 merged commit e42f23c into ElektraInitiative:master Jun 15, 2019
@markus2330
Copy link
Contributor

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants