dist/tools: add lazysponge tool#9634
Conversation
jcarrano
left a comment
There was a problem hiding this comment.
Almost Ok. I don't expect this tool to be used with large files, so I didn't mind checking efficiency.
dist/tools/lazysponge/lazysponge.py
Outdated
|
|
||
| # No support for 'interactive' input as catching Ctrl+c breaks in 'read' | ||
| if os.isatty(sys.stdin.fileno()): | ||
| print('Interactive input no supported. Use piped input') |
There was a problem hiding this comment.
This should be printed to stderr.
Makefile.include
Outdated
| @mkdir -p '$(dir $@)' | ||
| $(Q)'$(RIOTTOOLS)/genconfigheader/genconfigheader.sh' '$@' $(CFLAGS_WITH_MACROS) | ||
| $(Q)'$(RIOTTOOLS)/genconfigheader/genconfigheader.sh' $(CFLAGS_WITH_MACROS) \ | ||
| | '$(RIOTTOOLS)/lazysponge/lazysponge.py' $(if $(filter 1,$(QUIET)),,--verbose) '$@' |
There was a problem hiding this comment.
Considering this tool will be used in more than one place, can '$(RIOTTOOLS)/lazysponge/lazysponge.py' be made into a variable "$(LAZYSPONGE)"?
There was a problem hiding this comment.
Then it would also need to be exported to be visible in Makefile.base or pkg.mk for example.
There was a problem hiding this comment.
If there is a variable, should I also add LSFLAGS ?
There was a problem hiding this comment.
My comment was more about the tedium of having to write '$(RIOTTOOLS)/lazysponge/lazysponge.py' each time.
There was a problem hiding this comment.
For me it's the same for $(if $(filter 1,$(QUIET)),,--verbose) that's why I would add LSFLAGS or any other name.
|
|
||
| def _print_hash_debug_info(outfilename, oldbytes, newbytes): | ||
| """Print debug information on hashs.""" | ||
| oldhash = hashlib.md5(oldbytes).hexdigest() if oldbytes is not None else '' |
There was a problem hiding this comment.
How is printing the hash useful? The sizes would probably give more information.
There was a problem hiding this comment.
It could have the same size and be different. I also kept the previous behavior as before.
And when trying changes a hash is easy enough to recognize for 5 runs in a row.
dist/tools/lazysponge/lazysponge.py
Outdated
| ' If the content is the same, file is not modified.') | ||
| PARSER = argparse.ArgumentParser(description=DESCRIPTION) | ||
| PARSER.add_argument('outfile', help='Output file') | ||
| PARSER.add_argument('--verbose', help='Verbose output', default=False, |
There was a problem hiding this comment.
Can there be a short option too ("-v")?
|
For the efficiency, I did not try either. I am more introducing the concept here of a script to write generic configuration file and force rebuild when we are currently not. |
dist/tools/lazysponge/lazysponge.py
Outdated
| oldhash = hashlib.md5(oldbytes).hexdigest() if oldbytes is not None else '' | ||
| newhash = hashlib.md5(newbytes).hexdigest() | ||
| if oldbytes == newbytes: | ||
| print('Keeping old {} ({})'.format(outfilename, oldhash)) |
There was a problem hiding this comment.
These should also go to stderr.
There was a problem hiding this comment.
I was not sure to put debug to stderr, according to:
it depends if it is considered a conventional output or a diagnostic output.
And as its with --verbose I am not sure.
I pushed a fixup to go in your direction anyway.
There was a problem hiding this comment.
For tools, I tend to consider:
stdout: machine-readable output of the command, that can be piped to another command.stderr: auxiliary/non essential output meant to be read by a human, so that it still shows up even if stdout is redirected.
For an example of a data-copying command, see dd:
$ dd if=/dev/zero of=hhhh bs=1k count=1 >/dev/null
1+0 records in
1+0 records out
1024 bytes (1.0 kB, 1.0 KiB) copied, 0.00027606 s, 3.7 MB/s
|
I fixed a typo in the error message and added a test. |
|
Can I rebase/squash ? |
|
@cladmi Please. |
Write stdin to <outfile> if it is different from the previous content. If data provided in stdin is the same as the data that was in <outfile>, it is not modified so `last modification date` is not changed.
Remove file management from `genconfigheader` script and use `lazysponge` in Makefile.include. Use --verbose option when in non QUIET building mode.
4f06318 to
cc63d2d
Compare
|
Done, now waiting for murdock. |
Contribution description
Add a tool that writes stdin to a file only if it would modify the file content.
The design and name is based on
moreutilstoolspongehttps://rentes.github.io/unix/utilities/2015/07/27/moreutils-package/#spongeThe usage of the tool in the build system would be to save build configuration variables in a file and rebuild make targets if and only if the build configuration changed.
So only checking the variables would be done at each compilation.
I also used this file for
genconfigheaderto remove the file management out of it and give an usage example.Todo
Issues/PRs references
Follow up to the #9623 PR where .PHONY is replaced by a dependency.
#9589