Adjust sed arguments for compatibility with BSD sed #177
Conversation
Makefile
Outdated
| @@ -276,7 +276,7 @@ rvfi_preserve_fns=-c_preserve rvfi_set_instr_packet \ | |||
| generated_definitions/c/riscv_rvfi_model_$(ARCH).c: $(SAIL_RVFI_SRCS) model/main.sail Makefile | |||
| mkdir -p generated_definitions/c | |||
| $(SAIL) $(rvfi_preserve_fns) $(SAIL_FLAGS) -O -Oconstant_fold -memo_z3 -c -c_include riscv_prelude.h -c_include riscv_platform.h -c_no_main $(SAIL_RVFI_SRCS) model/main.sail -o $(basename $@) | |||
| sed -i -e '/^[[:space:]]*$$/d' $@ | |||
| sed -i.bak -e '/^[[:space:]]*$$/d' $@ | |||
There was a problem hiding this comment.
| sed -i.bak -e '/^[[:space:]]*$$/d' $@ | |
| sed -i '' -e '/^[[:space:]]*$$/d' $@ |
If a zero-length extension is given, no backup will be saved.
There was a problem hiding this comment.
I would double check if -i '' works with GNU sed, I think it may not. -i'' would without the space I think
There was a problem hiding this comment.
They're mutually incompatible, unfortunately.
There was a problem hiding this comment.
Perhaps you could use perl to do the same thing? I guess most systems will have it and it might be more portable?
There was a problem hiding this comment.
sed -e '/^[[:space:]]*$$/d' $@ > $@.new
mv $@.new $@
is a common portable way to get around the -i importability
There was a problem hiding this comment.
On second thoughts using sed and having the backups is probably best.
There was a problem hiding this comment.
@jrtc27 's method is the only true POSIX way... in any case, this discussion has convinced me that a comment is necessary to avoid future pain.
|
|
|
|
Right, sorry. Brain fart. |
|
While it looks good to me, we have 5 reviewers already assigned ... anyone willing to approve? |
|
I've redone it in the redirect and move style, on top of the current HEAD. |
|
It would be useful if someone with a BSDish system could test it. |
|
@jrtc27 Could you test on one of your BSD systems? |
Allows for (e.g.) BSD sed, which uses -i differently.
|
Rebased because the last one became messed up somehow (maybe I did it against the wrong branch?). Also tested it on a CheriBSD machine. |
We don't need the generated backup files, but it appears to be the only
way to be compatible with both GNU and BSD sed.