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

Correct compilation instructions for *nix #2026

Closed
wants to merge 3 commits into from

Conversation

akkartik
Copy link
Contributor

It looks like the old automagic file is no longer in the repo.

Copy link
Contributor

@MikuAuahDark MikuAuahDark left a comment

Choose a reason for hiding this comment

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

The wordings before the command probably can be made better.

readme.md Outdated Show resolved Hide resolved
akkartik and others added 3 commits February 26, 2024 09:28
It looks like the old `automagic` file is no longer in the repo.
Co-authored-by: Miku AuahDark <auahdark687291@gmail.com>
@akkartik
Copy link
Contributor Author

How does this look?

@MikuAuahDark
Copy link
Contributor

I'm still not too positive with the wording at "First pick a directory to store generated files to, say build/ and then:". Probably that's just me. Perhaps anyone else have better opinion?

@akkartik
Copy link
Contributor Author

Yeah, I'm not attached to it. Feel free to suggest something. Do you want it to be more explicit? Basically my thinking/preference was the commands are already saying what to do. But happy to add whatever you like.


When using a source release, automagic has already been run, and the first step can be skipped.
$ cmake -B build -S. --install-prefix $PWD/prefix # this will create the directory `build/`
$ cmake --build build --target install # this will put the files in `prefix/`.

Choose a reason for hiding this comment

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

build and prefix folders will popup as untracked files in git repository. Is it ok ?

Choose a reason for hiding this comment

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

cmake -S. also implies that we run these commands from the repository root. Probably we should explicitly mention it

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, you're right. The autotools build system used to allow in-tree build. With the transition to CMake, we explicitly forbid in-tree build.

Choose a reason for hiding this comment

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

As I see in CMakeLists.txt, you can't run cmake -B. -S., but it should be ok for cmake -Bbuild -S.

@@ -54,13 +54,10 @@ Compilation
Follow the instructions at the [megasource][megasource] repository page.

### *nix
Run `platform/unix/automagic` from the repository root, then run ./configure and make.
First pick a directory to store generated files to, say `build/` and then:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
First pick a directory to store generated files to, say `build/` and then:
Because in-tree builds are not allowed, you have to create Makefiles in a build folder.
In this example we will use the `build/` folder:

@MikuAuahDark
Copy link
Contributor

I opted slightly different text based on this PR. Thanks.

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

Successfully merging this pull request may close these issues.

None yet

4 participants