Skip to content

Commit 80a7e26

Browse files
catamorphismcatamorphism
committed
rustpkg: Tests for well-formed and ill-formed package IDs...
...and cleanup, making how we handle version numbers more rational (specifically, not passing in a versioned name to rustc with the -o flag), and removing unused code.
1 parent c3875e8 commit 80a7e26

File tree

5 files changed

+121
-271
lines changed

5 files changed

+121
-271
lines changed

‎src/librustpkg/conditions.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,7 @@ condition! {
2828
condition! {
2929
missing_pkg_files: (super::PkgId) -> ();
3030
}
31+
32+
condition! {
33+
bad_pkg_id: (super::Path, ~str) -> ::util::PkgId;
34+
}

‎src/librustpkg/path_util.rs‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ pub fn make_dir_rwx(p: &Path) -> bool { os::make_dir(p, u_rwx) }
3535

3636
/// True if there's a directory in <workspace> with
3737
/// pkgid's short name
38-
pub fn workspace_contains_package_id(pkgid: PkgId, workspace: &Path) -> bool {
38+
pub fn workspace_contains_package_id(pkgid: &PkgId, workspace: &Path) -> bool {
3939
let pkgpath = workspace.push("src").push(pkgid.local_path.to_str());
4040
os::path_is_dir(&pkgpath)
4141
}
@@ -67,17 +67,17 @@ pub fn built_executable_in_workspace(pkgid: &PkgId, workspace: &Path) -> Option<
6767

6868
/// Figure out what the test name for <pkgid> in <workspace>'s build
6969
/// directory is, and if the file exists, return it.
70-
pub fn built_test_in_workspace(pkgid: PkgId, workspace: &Path) -> Option<Path> {
70+
pub fn built_test_in_workspace(pkgid: &PkgId, workspace: &Path) -> Option<Path> {
7171
output_in_workspace(pkgid, workspace, Test)
7272
}
7373

7474
/// Figure out what the test name for <pkgid> in <workspace>'s build
7575
/// directory is, and if the file exists, return it.
76-
pub fn built_bench_in_workspace(pkgid: PkgId, workspace: &Path) -> Option<Path> {
76+
pub fn built_bench_in_workspace(pkgid: &PkgId, workspace: &Path) -> Option<Path> {
7777
output_in_workspace(pkgid, workspace, Bench)
7878
}
7979

80-
fn output_in_workspace(pkgid: PkgId, workspace: &Path, what: OutputType) -> Option<Path> {
80+
fn output_in_workspace(pkgid: &PkgId, workspace: &Path, what: OutputType) -> Option<Path> {
8181
let mut result = workspace.push("build");
8282
// should use a target-specific subdirectory
8383
result = mk_output_path(what, pkgid, &result);
@@ -94,7 +94,7 @@ fn output_in_workspace(pkgid: PkgId, workspace: &Path, what: OutputType) -> Opti
9494

9595
/// Figure out what the library name for <pkgid> in <workspace>'s build
9696
/// directory is, and if the file exists, return it.
97-
pub fn built_library_in_workspace(pkgid: PkgId, workspace: &Path) -> Option<Path> {
97+
pub fn built_library_in_workspace(pkgid: &PkgId, workspace: &Path) -> Option<Path> {
9898
let result = mk_output_path(Lib, pkgid, &workspace.push("build"));
9999
debug!("built_library_in_workspace: checking whether %s exists",
100100
result.to_str());
@@ -177,28 +177,27 @@ pub fn target_library_in_workspace(pkgid: &PkgId, workspace: &Path) -> Path {
177177
/// Returns the test executable that would be installed for <pkgid>
178178
/// in <workspace>
179179
/// note that we *don't* install test executables, so this is just for unit testing
180-
pub fn target_test_in_workspace(pkgid: PkgId, workspace: &Path) -> Path {
180+
pub fn target_test_in_workspace(pkgid: &PkgId, workspace: &Path) -> Path {
181181
target_file_in_workspace(pkgid, workspace, Test)
182182
}
183183

184184
/// Returns the bench executable that would be installed for <pkgid>
185185
/// in <workspace>
186186
/// note that we *don't* install bench executables, so this is just for unit testing
187-
pub fn target_bench_in_workspace(pkgid: PkgId, workspace: &Path) -> Path {
187+
pub fn target_bench_in_workspace(pkgid: &PkgId, workspace: &Path) -> Path {
188188
target_file_in_workspace(pkgid, workspace, Bench)
189189
}
190190

191191
fn target_file_in_workspace(pkgid: &PkgId, workspace: &Path,
192192
what: OutputType) -> Path {
193193
use conditions::bad_path::cond;
194194

195-
let (subdir, create_dir) = match what {
195+
let subdir = match what {
196196
Lib => "lib", Main | Test | Bench => "bin"
197197
};
198198
let result = workspace.push(subdir);
199-
debug!("target_file_in_workspace: %s %?", result.to_str(), create_dir);
200199
if !os::path_exists(&result) && !mkdir_recursive(&result, u_rwx) {
201-
cond.raise((result, fmt!("I couldn't create the %s dir", subdir)));
200+
cond.raise((copy result, fmt!("I couldn't create the %s dir", subdir)));
202201
}
203202
mk_output_path(what, pkgid, &result)
204203
}
@@ -222,17 +221,19 @@ pub fn build_pkg_id_in_workspace(pkgid: &PkgId, workspace: &Path) -> Path {
222221

223222
/// Return the output file for a given directory name,
224223
/// given whether we're building a library and whether we're building tests
225-
pub fn mk_output_path(what: OutputType, pkg_id: PkgId, workspace: &Path) -> Path {
226-
let short_name = pkg_id.short_name_with_version();
224+
pub fn mk_output_path(what: OutputType, pkg_id: &PkgId, workspace: &Path) -> Path {
225+
let short_name_with_version = pkg_id.short_name_with_version();
227226
// Not local_path.dir_path()! For package foo/bar/blat/, we want
228227
// the executable blat-0.5 to live under blat/
229228
let dir = workspace.push_rel(&*pkg_id.local_path);
230229
debug!("mk_output_path: short_name = %s, path = %s",
231-
short_name, dir.to_str());
230+
if what == Lib { copy short_name_with_version } else { copy pkg_id.short_name },
231+
dir.to_str());
232232
let output_path = match what {
233233
// this code is duplicated from elsewhere; fix this
234-
Lib => dir.push(os::dll_filename(short_name)),
235-
_ => dir.push(fmt!("%s%s%s", short_name,
234+
Lib => dir.push(os::dll_filename(short_name_with_version)),
235+
// executable names *aren't* versioned
236+
_ => dir.push(fmt!("%s%s%s", copy pkg_id.short_name,
236237
match what {
237238
Test => "test",
238239
Bench => "bench",

‎src/librustpkg/rustpkg.rc‎

Lines changed: 16 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -157,27 +157,6 @@ impl<'self> PkgScript<'self> {
157157
impl Ctx {
158158

159159
fn run(&self, cmd: ~str, args: ~[~str]) {
160-
let root = util::root();
161-
162-
util::need_dir(&root);
163-
util::need_dir(&root.push(~"work"));
164-
util::need_dir(&root.push(~"lib"));
165-
util::need_dir(&root.push(~"bin"));
166-
util::need_dir(&root.push(~"tmp"));
167-
168-
fn sep_name_vers(in: ~str) -> (Option<~str>, Option<~str>) {
169-
let mut name = None;
170-
let mut vers = None;
171-
172-
for str::each_split_char(in, '@') |s| {
173-
if name.is_none() { name = Some(s.to_owned()); }
174-
else if vers.is_none() { vers = Some(s.to_owned()); }
175-
else { break; }
176-
}
177-
178-
(name, vers)
179-
}
180-
181160
match cmd {
182161
~"build" => {
183162
if args.len() < 1 {
@@ -227,9 +206,7 @@ impl Ctx {
227206
return usage::uninstall();
228207
}
229208

230-
let (name, vers) = sep_name_vers(copy args[0]);
231-
232-
self.prefer(name.get(), vers);
209+
self.prefer(args[0], None);
233210
}
234211
~"test" => {
235212
self.test();
@@ -239,20 +216,16 @@ impl Ctx {
239216
return usage::uninstall();
240217
}
241218

242-
let (name, vers) = sep_name_vers(copy args[0]);
243-
244-
self.uninstall(name.get(), vers);
219+
self.uninstall(args[0], None);
245220
}
246221
~"unprefer" => {
247222
if args.len() < 1 {
248223
return usage::uninstall();
249224
}
250225

251-
let (name, vers) = sep_name_vers(copy args[0]);
252-
253-
self.unprefer(name.get(), vers);
226+
self.unprefer(args[0], None);
254227
}
255-
_ => fail!("reached an unhandled command")
228+
_ => fail!(fmt!("I don't know the command `%s`", cmd))
256229
}
257230
}
258231

@@ -267,7 +240,7 @@ impl Ctx {
267240
debug!("Destination dir = %s", build_dir.to_str());
268241

269242
// Create the package source
270-
let mut src = PkgSrc::new(workspace, &build_dir, &pkgid);
243+
let mut src = PkgSrc::new(workspace, &build_dir, pkgid);
271244
debug!("Package src = %?", src);
272245

273246
// Is there custom build logic? If so, use it
@@ -305,7 +278,6 @@ impl Ctx {
305278
// Build it!
306279
src.build(&build_dir, cfgs, self.sysroot_opt);
307280
}
308-
309281
}
310282

311283
fn clean(&self, workspace: &Path, id: &PkgId) {
@@ -317,7 +289,7 @@ impl Ctx {
317289
util::note(fmt!("Cleaning package %s (removing directory %s)",
318290
id.to_str(), dir.to_str()));
319291
if os::path_exists(&dir) {
320-
util::remove_dir_r(&dir);
292+
os::remove_dir_recursive(&dir);
321293
util::note(fmt!("Removed directory %s", dir.to_str()));
322294
}
323295

@@ -353,19 +325,19 @@ impl Ctx {
353325
debug!("Copying: %s -> %s", exec.to_str(), target_exec.to_str());
354326
if !(os::mkdir_recursive(&target_exec.dir_path(), u_rwx) &&
355327
os::copy_file(exec, &target_exec)) {
356-
cond.raise((*exec, target_exec));
328+
cond.raise((copy *exec, copy target_exec));
357329
}
358330
}
359331
for maybe_library.each |lib| {
360332
debug!("Copying: %s -> %s", lib.to_str(), target_lib.to_str());
361333
if !(os::mkdir_recursive(&target_lib.dir_path(), u_rwx) &&
362334
os::copy_file(lib, &target_lib)) {
363-
cond.raise((*lib, target_lib));
335+
cond.raise((copy *lib, copy target_lib));
364336
}
365337
}
366338
}
367339

368-
fn prefer(&self, _id: ~str, _vers: Option<~str>) {
340+
fn prefer(&self, _id: &str, _vers: Option<~str>) {
369341
fail!(~"prefer not yet implemented");
370342
}
371343

@@ -374,15 +346,16 @@ impl Ctx {
374346
fail!("test not yet implemented");
375347
}
376348

377-
fn uninstall(&self, _id: ~str, _vers: Option<~str>) {
349+
fn uninstall(&self, _id: &str, _vers: Option<~str>) {
378350
fail!("uninstall not yet implemented");
379351
}
380352

381-
fn unprefer(&self, _id: ~str, _vers: Option<~str>) {
353+
fn unprefer(&self, _id: &str, _vers: Option<~str>) {
382354
fail!("unprefer not yet implemented");
383355
}
384356
}
385357

358+
386359
pub fn main() {
387360
io::println("WARNING: The Rust package manager is experimental and may be unstable");
388361

@@ -443,32 +416,6 @@ pub struct Crate {
443416
cfgs: ~[~str]
444417
}
445418

446-
pub struct Listener {
447-
cmds: ~[~str],
448-
cb: ~fn()
449-
}
450-
451-
pub fn run(listeners: ~[Listener]) {
452-
let rcmd = copy os::args()[2];
453-
let mut found = false;
454-
455-
for listeners.each |listener| {
456-
for listener.cmds.each |&cmd| {
457-
if cmd == rcmd {
458-
(listener.cb)();
459-
460-
found = true;
461-
462-
break;
463-
}
464-
}
465-
}
466-
467-
if !found {
468-
os::set_exit_status(42);
469-
}
470-
}
471-
472419
pub impl Crate {
473420

474421
fn new(p: &Path) -> Crate {
@@ -527,10 +474,6 @@ pub fn src_dir() -> Path {
527474
os::getcwd()
528475
}
529476

530-
condition! {
531-
bad_pkg_id: (super::Path, ~str) -> ::util::PkgId;
532-
}
533-
534477
// An enumeration of the unpacked source of a package workspace.
535478
// This contains a list of files found in the source workspace.
536479
pub struct PkgSrc {
@@ -576,7 +519,7 @@ impl PkgSrc {
576519

577520
if !os::path_exists(&dir) {
578521
if !self.fetch_git() {
579-
cond.raise((self.id, ~"supplied path for package dir does not \
522+
cond.raise((copy self.id, ~"supplied path for package dir does not \
580523
exist, and couldn't interpret it as a URL fragment"));
581524
}
582525
}
@@ -598,12 +541,12 @@ impl PkgSrc {
598541
let mut local = self.root.push("src");
599542
local = local.push(self.id.to_str());
600543
// Git can't clone into a non-empty directory
601-
util::remove_dir_r(&local);
544+
os::remove_dir_recursive(&local);
602545

603546
let url = fmt!("https://%s", self.id.remote_path.to_str());
604547
util::note(fmt!("git clone %s %s", url, local.to_str()));
605548

606-
if run::program_output(~"git", ~[~"clone", url, local.to_str()]).status != 0 {
549+
if run::program_output(~"git", ~[~"clone", copy url, local.to_str()]).status != 0 {
607550
util::note(fmt!("fetching %s failed: can't clone repository", url));
608551
return false;
609552
}
@@ -733,3 +676,4 @@ impl PkgSrc {
733676
self.build_crates(maybe_sysroot, dst_dir, &dir, self.benchs, cfgs, Bench);
734677
}
735678
}
679+

0 commit comments

Comments
 (0)