dist/tools: add lazysponge tool #9634

cladmi · 2018-07-25T13:16:00Z

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 moreutils tool sponge https://rentes.github.io/unix/utilities/2015/07/27/moreutils-package/#sponge

The 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 genconfigheader to remove the file management out of it and give an usage example.

Todo

Add python tests

Issues/PRs references

Follow up to the #9623 PR where .PHONY is replaced by a dependency.
#9589

jcarrano

Almost Ok. I don't expect this tool to be used with large files, so I didn't mind checking efficiency.

jcarrano · 2018-07-25T13:23:16Z

dist/tools/lazysponge/lazysponge.py

+
+    # 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')


This should be printed to stderr.

jcarrano · 2018-07-25T13:27:12Z

Makefile.include

@@ -646,7 +646,8 @@ include $(RIOTTOOLS)/desvirt/Makefile.desvirt
 # The script will only touch the file if anything has changed since last time.
 $(RIOTBUILD_CONFIG_HEADER_C): FORCE
 	@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) '$@'


Considering this tool will be used in more than one place, can '$(RIOTTOOLS)/lazysponge/lazysponge.py' be made into a variable "$(LAZYSPONGE)"?

Then it would also need to be exported to be visible in Makefile.base or pkg.mk for example.

If there is a variable, should I also add LSFLAGS ?

My comment was more about the tedium of having to write '$(RIOTTOOLS)/lazysponge/lazysponge.py' each time.

For me it's the same for $(if $(filter 1,$(QUIET)),,--verbose) that's why I would add LSFLAGS or any other name.

jcarrano · 2018-07-25T13:32:20Z

dist/tools/lazysponge/lazysponge.py

+
+def _print_hash_debug_info(outfilename, oldbytes, newbytes):
+    """Print debug information on hashs."""
+    oldhash = hashlib.md5(oldbytes).hexdigest() if oldbytes is not None else ''


How is printing the hash useful? The sizes would probably give more information.

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.

jcarrano · 2018-07-25T13:32:59Z

dist/tools/lazysponge/lazysponge.py

+               ' 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,


Can there be a short option too ("-v")?

cladmi · 2018-07-31T13:37:09Z

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.
If there is indeed an efficiency issue for starting python at some point it could be re-implemented in C with the same interface.

jcarrano · 2018-08-01T13:20:03Z

dist/tools/lazysponge/lazysponge.py

+    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))


These should also go to stderr.

I was not sure to put debug to stderr, according to:

https://unix.stackexchange.com/questions/331611/do-progress-reports-logging-information-belong-on-stderr-or-stdout

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.

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

cladmi · 2018-08-09T13:25:48Z

I fixed a typo in the error message and added a test.

cladmi · 2018-08-20T09:26:43Z

Can I rebase/squash ?

jcarrano · 2018-08-20T09:27:48Z

@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.

cladmi · 2018-08-20T09:37:02Z

Done, now waiting for murdock.

cladmi added Area: build system Area: Build system Area: tools Area: Supplementary tools labels Jul 25, 2018

cladmi requested review from jnohlgard, jcarrano and kaspar030 July 25, 2018 13:16

jcarrano mentioned this pull request Jul 25, 2018

Makefiles: Remove abuses of FORCE and .PHONY #9631

Closed

3 tasks

jcarrano suggested changes Jul 25, 2018

View reviewed changes

jcarrano reviewed Aug 1, 2018

View reviewed changes

cladmi mentioned this pull request Aug 6, 2018

pkg/ccn-lite/relic: check for minimal cmake version #9720

Merged

cladmi added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 7, 2018

jcarrano approved these changes Aug 7, 2018

View reviewed changes

jcarrano approved these changes Aug 20, 2018

View reviewed changes

cladmi added 3 commits August 20, 2018 11:34

dist/tools: add lazysponge tool

0528e09

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.

make: add LAZYSPONGE and LAZYSPONGE_FLAGS variables

94f2a49

genconfigheader: use lazysponge for file management

cc63d2d

Remove file management from `genconfigheader` script and use `lazysponge` in Makefile.include. Use --verbose option when in non QUIET building mode.

cladmi force-pushed the pr/make/lazysponge branch from 4f06318 to cc63d2d Compare August 20, 2018 09:35

jcarrano merged commit 635ecf9 into RIOT-OS:master Aug 20, 2018

cladmi deleted the pr/make/lazysponge branch August 20, 2018 12:10

cladmi mentioned this pull request Sep 20, 2018

Makefile.include: replace $(USEPKG:%=$(BINDIR)/%.a) target #9935

Merged

jcarrano mentioned this pull request Sep 27, 2018

[RFC] [PoC] Configuration generation #10058

Closed

jcarrano mentioned this pull request Nov 2, 2018

tools/mkconstfs2: Make templates configurable. #9565

Closed

cladmi added this to the Release 2018.10 milestone Nov 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dist/tools: add lazysponge tool #9634

dist/tools: add lazysponge tool #9634

cladmi commented Jul 25, 2018 •

edited

Loading

jcarrano left a comment

jcarrano Jul 25, 2018

jcarrano Jul 25, 2018

cladmi Jul 31, 2018

cladmi Jul 31, 2018

jcarrano Aug 1, 2018

cladmi Aug 7, 2018

jcarrano Jul 25, 2018

cladmi Jul 31, 2018

jcarrano Jul 25, 2018

cladmi commented Jul 31, 2018 •

edited

Loading

jcarrano Aug 1, 2018

cladmi Aug 7, 2018

jcarrano Aug 8, 2018

cladmi commented Aug 9, 2018

cladmi commented Aug 20, 2018

jcarrano commented Aug 20, 2018

cladmi commented Aug 20, 2018

dist/tools: add lazysponge tool #9634

dist/tools: add lazysponge tool #9634

Conversation

cladmi commented Jul 25, 2018 • edited Loading

Contribution description

Todo

Issues/PRs references

jcarrano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cladmi commented Jul 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cladmi commented Aug 9, 2018

cladmi commented Aug 20, 2018

jcarrano commented Aug 20, 2018

cladmi commented Aug 20, 2018

cladmi commented Jul 25, 2018 •

edited

Loading

cladmi commented Jul 31, 2018 •

edited

Loading