[PATCH] D106052: [flang][driver] Randomise the names of the temporary unparsed files

[David Spickett via Phabricator via llvm-commits]

DavidSpickett added inline comments.


================
Comment at: flang/tools/f18/flang.in:331
+  # invocation. Otherwise we can't use this script in parallel.
+  RANDOM=$(date +%N | cut -b4-9)
+  local -r unparsed_file_base="flang_unparsed_source_file_$RANDOM"
----------------
DavidSpickett wrote:
> awarzynski wrote:
> > tschuett wrote:
> > > awarzynski wrote:
> > > > DavidSpickett wrote:
> > > > > Wouldn't date/time have a small chance to collide?
> > > > > 
> > > > > Would something like `/proc/sys/kernel/random/uuid` be better to guarantee that each one is unique?
> > > > > Wouldn't date/time have a small chance to collide?
> > > > 
> > > > I'm also slightly concerned, but `%N` gives you nanoseconds and I'd hope that that's enough to get rando results.
> > > > 
> > > > > Would something like `/proc/sys/kernel/random/uuid` be better to guarantee that each one is unique?
> > > > 
> > > > I like your suggestion! Are we confident that it's going to be present on every system that this script might be run on? I think that it would be OK to limit it to Linux, but I would need to check with @sscalpone (he is likely to use this script a lot).
> > > How about `mktemp`? At least on Apple proc does not exists. I doubt Windows will have a proc dir.
> > This one is also nice @tschuett , thanks! However, the version that I have on Darwin only allows me to specify the prefix. I need to be able to specify the suffix though:
> > ```
> > mktemp XXXXXX.f90
> > ```
> > I can do this with `mktemp` on Linux. 
> > 
> > Perhaps it's worth adding a bit of context  here. This script is only a temporary workaround and hence there is no need to support every possible system or to make it future-proof. It is fine to use something that's OS specific, as long as we support all our users that rely on this script. AFAIK, the list is not long. And I assume that all of Linaro's BuildBots are running Linux.
> > 
> > As for `/proc/sys/kernel/random/uuid`, I just want to make sure that it's available on all Linux systems. Given the location, it's seems safe to assume that that's the case, right?
> I see some isolated comments that it's not present on some Linux, but it's listed on the man page:
> https://man7.org/linux/man-pages/man4/random.4.html
> (maybe they were talking about older kernels)
> 
> I have `uuidgen` on Ubuntu, maybe that's a better way to go if Macs also have that. (since they and the BSDs won't have /proc)
> And I assume that all of Linaro's BuildBots are running Linux.

All recent Ubuntu yes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106052/new/

https://reviews.llvm.org/D106052