UUID and sha256 digest functions #1170

mbodmer · 2022-05-01T21:10:07Z

issue #1018 mentioned a uuid function would be helpful.

Here is my attempt to deliver this. Also I've included sha256 hashing functions for strings and files.

To create a meaningful unittest for uuid() I had to refactor the Test struct in test.rs, I hope this is acceptable. The challenge was to keep the current interface but allow for custom verification. Well, here is my attempt. I'm new to rust, so bear with me.

mbodmer · 2022-05-02T17:10:17Z

#1011

casey · 2022-05-03T02:14:32Z

Thanks for this PR! There's already Test::stderr_regex, and I think it might be cleaner to add a Test::stdout_regex to test that the output matches a pattern, instead of adding a new kind of validator. for UUIDs, it should be okay to check that it matches the pattern /........-....-....-....-............/

For the hash functions, can you use the implementation from https://github.com/RustCrypto/hashes? I think it's better maintained.

mbodmer · 2022-05-03T06:20:47Z

Hi, thanks for your review. I have now added stdout_regex in the sense of the existing stderr_regex. I agree, much less complexity that way. Also I have replaced the hashing dependency with rust-crypto. Hope this helps.

mbodmer · 2022-05-03T06:23:28Z

I'm not sure whether the general name digest()/digest_file() for the functions is too broad. Any requests for a better more concise name? :-)
eg: sha256()/sha256_file()

casey · 2022-05-03T06:35:34Z

I'm not sure whether the general name digest()/digest_file() for the functions is too broad. Any requests for a better more concise name? :-) eg: sha256()/sha256_file()

I think I agree, sha256() and sha256_file() are probably better, so if you're reading a justfile and you don't know the function you can see what it does without having to look up documentation.

mbodmer · 2022-05-03T10:02:39Z

Ok, they are now renamed to the more specific sha256(), sha256_file()

casey

Nice, looking good. I added a few minor comments.

casey · 2022-05-04T05:22:18Z

README.md

@@ -1016,6 +1016,12 @@ These functions can fail, for example if a path does not have an extension, whic

 - `error(message)` - Abort execution and report error `message` to user.

+#### UUID and Hash Generation
+
+- `sha256(textdata)` Generates a sha256 hash of the given text data. Example: `sha256("just hello")`


Suggested change

- `sha256(textdata)` Generates a sha256 hash of the given text data. Example: `sha256("just hello")`

- `sha256(string)` Return the SHA-256 hash of `string` as a hexadecimal string.

casey · 2022-05-04T05:22:39Z

README.md

+#### UUID and Hash Generation
+
+- `sha256(textdata)` Generates a sha256 hash of the given text data. Example: `sha256("just hello")`
+- `sha256_file(path)` Generates a sha256 hash of the given file. Example: `sha256_file("/path/to/file")`


Suggested change

- `sha256_file(path)` Generates a sha256 hash of the given file. Example: `sha256_file("/path/to/file")`

- `sha256_file(path)` Return the SHA-256 hash of the file at `path` as a hexadecimal string.

casey · 2022-05-04T05:22:56Z

README.md

+
+- `sha256(textdata)` Generates a sha256 hash of the given text data. Example: `sha256("just hello")`
+- `sha256_file(path)` Generates a sha256 hash of the given file. Example: `sha256_file("/path/to/file")`
+- `uuid()` - Randomly generates a hyphenated UUID string.


Suggested change

- `uuid()` - Randomly generates a hyphenated UUID string.

- `uuid()` - Return a randomly generated UUID.

casey · 2022-05-04T05:25:11Z

src/function.rs

+  let mut file = std::fs::File::open(path).unwrap();
+  std::io::copy(&mut file, &mut hasher).unwrap();


These unwraps should both be handled. Something like:

Suggested change

let mut file = std::fs::File::open(path).unwrap();

std::io::copy(&mut file, &mut hasher).unwrap();

let mut file = std::fs::File::open(path).map_err(|err| format!("Failed to open file at `{}`: {}", path, err))?;

std::io::copy(&mut file, &mut hasher).map_err(|err| format!("Failed to read file at `{}`: {}", path, err)?;

casey · 2022-05-04T05:25:56Z

tests/functions.rs

+#[test]
+fn sha256_file() {
+  let mut tmpfile = tempfile::NamedTempFile::new().unwrap();
+  writeln!(tmpfile, "just is great").unwrap();


casey · 2022-05-04T05:27:18Z

tests/functions.rs

+  let justfile_content = format!("x := sha256_file('{}')", tmppath.to_str().unwrap());
+
+  Test::new()
+    .justfile(justfile_content)


You can use the somewhat odd tree function to populate the tempdir with files:

Suggested change

.justfile(justfile_content)

.justfile("x := sha256_file('file')")

.tree(tree! {

file: "just is great",

})

mbodmer · 2022-05-04T18:05:16Z

Thanks for the extensive review, I think I've covered all your issues now, feel free to check again

Signed-off-by: mbodmer <[email protected]>

Signed-off-by: Marc Bodmer <[email protected]>

according to casey's review Signed-off-by: mbodmer <[email protected]>

casey

Nice, looks good, although I just realized that there's a bug. just doesn't change the working directory, so you'll need to join the path in sha256_file with context.search.working_directory, i.e. context.search.working_directory.join(path).

Could you also add a test for this? It should fail on the current PR, and then pass once path is joined with the working directory. You can use Test::current_dir to make just start in a sub-directory of the tmpdir. You can try to do sha256_file on a file in tempdir.

mbodmer · 2022-05-04T20:25:24Z

Sorry for force pushing, I've had to rewrite the history to rebase the branch, since you've also modified the lockfile and stupid me checked it in, in a commit together with other changes.
Can you give me more details, I don't understand where, how and what causes the bug you've mentioned.

casey · 2022-05-04T20:38:38Z

Lets say you have the following files:

/justfile
/bar
/foo/

And you're currently in /foo/. /justfile contains:

baz:
  echo {{sha256_file("bar")}}}

You do:

$ just

The just executable looks for the justfile and sees that it's in /, so it sets search.working_directory to /. However, it does not call env::set_current_dir with /, so the current directory is still /bar. So then sha256_file("bar") will fail, because there is no /foo/bar.

mbodmer · 2022-05-04T20:48:11Z

Ok, I think now I understand, thanks. Will try

Signed-off-by: mbodmer <[email protected]>

src/function.rs

casey

Looks good! I went ahead and removed a _ that is now unnecessary.

mbodmer · 2022-05-05T05:19:44Z

Thanks

mbodmer force-pushed the uuid-function branch from 77df3db to 9536609 Compare May 3, 2022 06:18

casey requested changes May 4, 2022

View reviewed changes

mbodmer requested a review from casey May 4, 2022 18:31

mbodmer force-pushed the uuid-function branch from b70c327 to d86c6ed Compare May 4, 2022 20:03

mbodmer and others added 9 commits May 4, 2022 22:10

Add uuid() function

4dcb4fd

Signed-off-by: mbodmer <[email protected]>

adding sha256 digest functions

64ab6f6

Signed-off-by: mbodmer <[email protected]>

add documentation for uuid and digest functions

14b21fd

Signed-off-by: mbodmer <[email protected]>

fix typo in doc

2fc0c17

Signed-off-by: mbodmer <[email protected]>

add stdout_regex to test.rs

5485b3e

Signed-off-by: mbodmer <[email protected]>

replace crate sha256 by sha2 of rustcrypto

9d57140

Signed-off-by: mbodmer <[email protected]>

renamed digest* functions to sha256*

e0f7c9d

Signed-off-by: Marc Bodmer <[email protected]>

fix readme doc on uuid()/sha256()/sha256_file()

44b0669

according to casey's review Signed-off-by: mbodmer <[email protected]>

correctly handle errors when opening file for sha256_file()

b9ede77

according to casey's review Signed-off-by: mbodmer <[email protected]>

mbodmer force-pushed the uuid-function branch from 4334514 to 1696499 Compare May 4, 2022 20:11

casey requested changes May 4, 2022

View reviewed changes

mbodmer added 3 commits May 4, 2022 22:50

update lockfile after rebase to 1.1.3

fd771db

Signed-off-by: mbodmer <[email protected]>

add failing test for shafile and workdir in subdirectory

5938dce

Signed-off-by: mbodmer <[email protected]>

fix path for sha256_file() by joining with justfile directory

d254284

Signed-off-by: mbodmer <[email protected]>

mbodmer force-pushed the uuid-function branch from 1696499 to d254284 Compare May 4, 2022 22:29

mbodmer requested a review from casey May 4, 2022 22:31

casey reviewed May 4, 2022

View reviewed changes

src/function.rs Outdated Show resolved Hide resolved

Update src/function.rs

bbfa0bb

casey approved these changes May 4, 2022

View reviewed changes

casey enabled auto-merge (squash) May 4, 2022 23:16

casey merged commit 862d6a5 into casey:master May 4, 2022

casey mentioned this pull request Jan 25, 2023

Add uuid() function #1018

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UUID and sha256 digest functions #1170

UUID and sha256 digest functions #1170

mbodmer commented May 1, 2022

mbodmer commented May 2, 2022

casey commented May 3, 2022

mbodmer commented May 3, 2022

mbodmer commented May 3, 2022

casey commented May 3, 2022

mbodmer commented May 3, 2022

casey left a comment

casey May 4, 2022

casey May 4, 2022

casey May 4, 2022

casey May 4, 2022

casey May 4, 2022

casey May 4, 2022

mbodmer commented May 4, 2022

casey left a comment

mbodmer commented May 4, 2022

casey commented May 4, 2022

mbodmer commented May 4, 2022

casey left a comment

mbodmer commented May 5, 2022

	- `sha256(textdata)` Generates a sha256 hash of the given text data. Example: `sha256("just hello")`
	- `sha256(string)` Return the SHA-256 hash of `string` as a hexadecimal string.

	- `sha256_file(path)` Generates a sha256 hash of the given file. Example: `sha256_file("/path/to/file")`
	- `sha256_file(path)` Return the SHA-256 hash of the file at `path` as a hexadecimal string.

	- `uuid()` - Randomly generates a hyphenated UUID string.
	- `uuid()` - Return a randomly generated UUID.

		let mut file = std::fs::File::open(path).unwrap();
		std::io::copy(&mut file, &mut hasher).unwrap();

-    .justfile(justfile_content)
+    .justfile("x := sha256_file('file')")
+    .tree(tree! {
+      file: "just is great",
+    })

UUID and sha256 digest functions #1170

UUID and sha256 digest functions #1170

Conversation

mbodmer commented May 1, 2022

mbodmer commented May 2, 2022

casey commented May 3, 2022

mbodmer commented May 3, 2022

mbodmer commented May 3, 2022

casey commented May 3, 2022

mbodmer commented May 3, 2022

casey left a comment

Choose a reason for hiding this comment

casey May 4, 2022

Choose a reason for hiding this comment

casey May 4, 2022

Choose a reason for hiding this comment

casey May 4, 2022

Choose a reason for hiding this comment

casey May 4, 2022

Choose a reason for hiding this comment

casey May 4, 2022

Choose a reason for hiding this comment

casey May 4, 2022

Choose a reason for hiding this comment

mbodmer commented May 4, 2022

casey left a comment

Choose a reason for hiding this comment

mbodmer commented May 4, 2022

casey commented May 4, 2022

mbodmer commented May 4, 2022

casey left a comment

Choose a reason for hiding this comment

mbodmer commented May 5, 2022