Correct Ubuntu build instructions #984
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
There was a problem hiding this comment.
@Orvid has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
README.md
Outdated
| @@ -141,12 +141,22 @@ sudo apt-get install \ | |||
|
|
|||
| In the folly directory, run: | |||
| ``` | |||
| mkdir _build && cd _build | |||
| mkdir _build | |||
There was a problem hiding this comment.
The line below is cmake ..; that makes this change incorrect.
The reason for building from within the _build dir is to avoid cmake polluting the top-level dir with build artifacts, which it otherwise would do.
There was a problem hiding this comment.
Hmm do you mean the top level directory that contains the .md files and the build, folly and CMake folders? No build artifacts were added there. It's within the folly folder that a few new files and folders were created.
More so, the commands just didn't work inside [folly version]/folly/_build. #847 contains the error I got from running cmake .. inside it, but both make -j $(nproc) and make install gave errors that indicated necessary files were missing, and then they worked when I went back from [folly version]/folly/_build/ to [folly version]/folly/ as @Orvid suggested.
There was a problem hiding this comment.
Right, top-level or folly/ subdir - we want to avoid build artifact pollution.
Was your shell in [folly-checkout]/folly/_build or in [folly-checkout]/_build when you ran cmake ..?
There was a problem hiding this comment.
It was in [folly-checkout]/folly/_build when I ran it and got the errors. Should the _build folder have been made in [folly-checkout]/ rather than [folly-checkout]/folly/? The README says
In the folly directory
so I interpreted that as [folly-checkout]/folly.
There was a problem hiding this comment.
The shell should be in [folly-checkout]/_build, and cmake .. should be run from that directory.
There was a problem hiding this comment.
Okay, thanks for clarifying. So what do you think of these changes:
In the root of the folly directory, run:
mkdir _build && cd _build # puts the shell in [folly-checkout]/_build
There was a problem hiding this comment.
I don't see that as a helpful comment because it merely restates the code into English.
There was a problem hiding this comment.
Okay, so only the first line then?
In the root of the folly directory, run:
There was a problem hiding this comment.
"The X directory" and "the root of the X directory" mean the same thing. However you might say
In the folly directory (e.g. the checkout root or the archive unpack root), run:
There was a problem hiding this comment.
Okay, thank you. I have updated the code.
README.md
Outdated
| > "/home/[yourname]/[folly-version]/folly/libfolly.a" to | ||
| > "/usr/local/lib/libfolly.a". | ||
|
|
||
| then you [need to use `sudo make install`](https://askubuntu.com/a/954877) |
There was a problem hiding this comment.
This link does not need to exist - this random answer to a random question is not any kind of canonical or authoritative source text.
There was a problem hiding this comment.
Okay, I will remove it.
README.md
Outdated
| > "/usr/local/lib/libfolly.a". | ||
|
|
||
| then you [need to use `sudo make install`](https://askubuntu.com/a/954877) | ||
| as the last step instead of just `make install`. |
There was a problem hiding this comment.
Not sold on this whole comment needing to exist. Installing things into the system is well-known to require admin privileges or the root user, and sudo is the well-known way to get those on Ubuntu.
Of course there are many people who don't know this, but it's not really about folly and it's more about getting familiar with the operating system.
At best, we can have an in-line comment suggesting either to use sudo or to use DESTDIR but without offering any explanation. E.g.:
make install # with either sudo or DESTDIR as necessary
There was a problem hiding this comment.
Alright, I will make the change.
There was a problem hiding this comment.
@yfeldblum has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Clarify the [last section](https://github.com/facebook/folly#ubuntu-1604-lts) of the Ubuntu build instructions about where commands need to be run from and how to invoke `make install`. Fixes facebook#847. Pull Request resolved: facebook#984 Reviewed By: Orvid Differential Revision: D13406491 Pulled By: yfeldblum fbshipit-source-id: d04121812d0d7394000ea6116745c236db67aa23
Associated issue: #847
The
cmakeandmakecommands given in the last section of the Ubuntu build instructions all need to be run from inside thefollyfolder rather than the_buildfolder, as indicated by @Orvid in #847. Also, the final step (make install) may need to besudo make installto allow access to the/usrfolder.