-
-
Notifications
You must be signed in to change notification settings - Fork 360
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
Add Rust to SCons #945
Add Rust to SCons #945
Conversation
Cooley-Tukey and Split Operator Method don't compile, but this is a problem with those files rather than SCons. I've already |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we shouldn't keep the src/
to not diverge too much from default Cargo.toml
settings. We still want to show good practices with Rust.
I see there are builders for both rustc
and cargo
, which is fine. But maybe it'll be simplest if we just keep the cargo
one?
|
||
[[bin]] | ||
path = "./monte_carlo.rs" | ||
name = "main" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use the file name to make things clearer.
name = "main" | |
name = "monte_carlo" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name determines what the name of the executable is. If it's not constant then it will need to be passed as another arguement to the build, and as it gets renamed automatically when it's moved I decided this would be simpler.
|
||
[[bin]] | ||
path = "./split_op.rs" | ||
name = "main" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name = "main" | |
name = "split_op" |
|
||
[[bin]] | ||
path = "./huffman.rs" | ||
name = "main" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name = "main" | |
name = "huffman" |
|
||
[[bin]] | ||
path = "./fft.rs" | ||
name = "main" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name = "main" | |
name = "fft" |
|
||
env['CCFLAGS'] = '' | ||
env['CXXFLAGS'] = '-std=c++17' | ||
env['ASFLAGS'] = '--64' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know scons
why is this needed for Rust?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's part of the assembly build. It might have got mixed up in a rebase
[[bin]] | ||
path = "./barnsley.rs" | ||
name = "main" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use the file name to make things clearer.
name = "main" | |
name = "barnsley" |
Dockerfile install looks fine to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I tested the Rust compilation with your docker file, and it works (except the two you pointed out in #944, of course).
I'm happy to merge this in when you are ready.
I've fixed the issues with some programs not compiling due to the wrong library versions being used. Solves #944 . It could do with updating, but that's another issue. I'm all ready for it to get merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can confirm it works in the docker container.
Happy to merge this :)
Add Rust to the SCons build system. If a Cargo.toml file is present then it builds with Cargo, otherwise it will build with rustc. As such a Cargo.toml file is necessary if any external crates are necessary (usally an
extern crate ...;
in the source file). This could be replaced with a customScanner
, but that's probably overkill and error prone.In order to have all the code files get picked up by SCons, I have the *.rs and the Cargo.toml file in the same directory, and define the path to the source file in Cargo.toml. An example of this is the Cooley-Tukey code.
I have the *.pdb files and the target/ directories marked by SCons for cleaning when using the
-c
argument. This deletes the Cargo.lock files, causing cargo to recompile the external crates on the next build.Due to Cargo putting the executable into a directory in the target directory, it is built in the same place as the source file and the executable then copied to the build/ directory. There is a command line argument to copy the build artifacts to a directory (--out-dir) but this is only available on nightly rust. When (and if) this gets stabilized then it would simplify the SConstruct file to take advantage of this.