mozilla :: #rust-infra

13 Jul 2017
00:20simulacrumacrichto: So I just noted that the Extended installer looks like it'll fail if everything isn't enabled (e.g, docs, extended build, etc.)
00:20simulacrumIs that intentional?
00:29aidanhsnot entirely sure why appveyor has started leaving messages on PRs -
00:30aidanhs('messages' meaning 'build statuses'
00:30simulacrumI'd guess something changed on their side
00:31aidanhsyeah, I assume so, bit distracting
00:33simulacrumlittle bit
00:33simulacrummaybe worth adding to or something as a note
04:22acrichtosimulacrum: heh 99% of bugs in rustbuild are "no one has ever run this configuration yet"
04:22acrichtoit's basically *supposed* to be as flexible as possible
04:22acrichtoif it's not then it's worth considering it a bug
15:20simulacrumacrichto: Can you prepare a patch for the beta? I don't have an easy way to check it out quite right now
15:20simulacrumShould just be removing the other library features from summary
15:20acrichtosimulacrum: yeah I'm fixing it
15:45simulacrumacrichto: So with the core intrinsic requirement, I came up with a solution that's painful but works:
15:45simulacrumWe'd have to declare type Id and type OutputId on every implementor of Step I think
15:45simulacrumcc aidanhs -- you were looking into this as well
15:46acrichtosimulacrum: sorry I haven't really dug deep into this, I don't understand the intricacies here
15:47simulacrumacrichto: We can discuss it if you'd like (probably later, triage in ~15 minutes) -- the summary is that we need some way to distinguish between each step implementor
15:48simulacrumwhether that's TypeId or type_name intrinsic or something doesn't matter
15:48acrichtosimulacrum: this is all just an implementation of `ensure`, right?
15:48simulacrumah, more like for the cache
15:49acrichtoif this gets too painful we should just use Rc instead of lifetimes so we can use vanilla Any
15:49simulacrumacrichto: yeah, we could
15:50simulacrumIt's a bit painful to do so (not on an implementation level) because it seems like this is where Rust's lifetimes could really shine
15:50acrichtosimulacrum: alternatively why not just to_json the key?
15:51acrichtowhy does the type_name need to be prepended?
15:51simulacrumserde_json doesn't encode the type in the key/value, so the structs we have (e.g. in dist, `struct Src;`) is serialized as null
15:52simulacrumWhich we can workaround by implementing Serialize ourselves (it's trivial) for those cases, maybe.
15:52simulacrumIIRC, though, Serde doesn't differentiate between structs with equivalent contents and names in differnet modules
15:52acrichtoah ok
15:52simulacrume.g. compile::Std and check::Std could presumably both have the same contents (compiler, target)
15:53acrichtoI just feel like we shouldn't be fighting the type system in the build system...
15:53acrichtothis is the place where we want like 0 cleverness
15:53aidanhsfyi my branch converts everything to owned data and uses vanilla Any
15:53acrichtooh ok, that seems fine yeah
15:53simulacrumYeah, vanilla Any seems fine, though isn't *really* a win
15:53simulacrumi.e. it's the same thing under the hood afaict
15:53aidanhsit's a bit sad seeing all the clones/to_owned
15:54simulacrumRealistically, as far as I can tell, if there&#39;s a solution for given S: MyTrait<&#39;a> to get S2: MyTrait<&#39;static> then we&#39;re good
15:54simulacrumI can do it with transmute -- no problem -- and it&#39;d technically be safe
17:13simulacrumaidanhs: This is my approach
17:22shepacrichto: &quot;export RUSTC_WRAPPER=sccache&quot;
17:22shepforget something?
17:23acrichtoshep: that should be it yeah
17:24aidanhssimulacrum: it&#39;s an improvement on before, I don&#39;t think there&#39;s really an ideal way to do this
17:25aidanhsfwiw though, the return values may as well not be json since we can&#39;t rely on borrowed deserialisation
17:25simulacrumWhy not?
17:26aidanhsyou can&#39;t deserialize some strings as borrowed, anything string with a &quot; or \ being obvious examples
17:26aidanhs(because they have to be escaped in json)
17:27simulacrumHow does Serde do this then?
17:27simulacrumWill it panic on me?
17:27aidanhsprobably just a failed deserialization. it&#39;s not a problem right now because you only return owned data
17:27simulacrumhm, well, no
17:27simulacrum-> Compiler<&#39;a>
17:28aidanhsehh, you&#39;re not going to get &quot; and \ in target triples
17:28aidanhsI&#39;m more thinking of paths
17:28aidanhsbut yes, you&#39;re right that that technically could fail someday
17:29simulacrumI don&#39;t really mind going with your branch, it&#39;s just painful to see all the copying when we shouldn&#39;t need it realistically
17:30simulacrumOr at least I don&#39;t think we should
17:31aidanhsI do agree, it is unfortunate. the killer issue is that you can&#39;t use a Step as a key while they&#39;re non-static. by using serialization it&#39;s effectively a &quot;clone-and-make-owned&quot;, which is cunning
17:32aidanhsI just wanted to put the option out there
17:32simulacrumyeah, thanks for writing it up so we can at least easily consider the trade-off
17:33shepacrichto: womp womp
17:33simulacrumI still don&#39;t really know why HashMap<TypeId, HashMap<ConcreteStep, CS::Output>> isn&#39;t possible to write -- it should be, right?
17:34simulacrumThis is basically generics-by-hashmap
17:35aidanhsyou don&#39;t know how long that ConcreteStep will last
17:35simulacrumI&#39;m fine with giving it a lifetime though
17:35simulacrumWe have the guarantee that all borrowed data will last long enough since it comes from Build, which we borrow
17:36aidanhsyou can&#39;t though. as soon as you put it inside a RefCell, lifetimes become invariant, so all your steps etc need to have identical lifetimes
17:36simulacrumYes, I know it doesn&#39;t work in practice, but it *should* -- right?
17:36simulacrumwell, or at least could
17:36acrichtoshep: :(
17:36aidanhssimulacrum: serde issue fyi
17:37aidanhsif you change L to have a String name instead it&#39;ll work
17:37simulacrumhuh, okay
17:38aidanhssimulacrum: ah no not really, it is actually unsound to have lifetimes be variant inside a refcell
17:38simulacrumaidanhs: Well, one simple solution to this: intern all targets and hosts, and store u32s, making the data copy, making everything simple
17:38aidanhsI refresh my memory with every now and again
17:39simulacrumsame with pathbuf, etc
17:39aidanhssimulacrum: the best string interning for rust (short of a &#39;string_to_static_str&#39; leaking method) is servo&#39;s string-cache
17:39aidanhswhich again requires clone
17:40aidanhs(because runtime interning is done with a refcount)
17:40aidanhsbut yes, I did consider string_to_static_str, it&#39;s totally viable
17:40simulacrumwell, we shouldn&#39;t need clone, right? Or at least, only once in theory -- to put it in the intern table
17:41aidanhsit&#39;s just the way string-cache works right now. there&#39;s nothing stopping us from using our own
17:41aidanhsbut these strings are so small I&#39;d probably just deliberately leak them as an initial attempt
17:42aidanhswe can intern properly down the line
17:42simulacrumeh, leaking the strs doesn&#39;t solve the problem though does it?
17:42aidanhsthink it does
17:42aidanhsall of your &#39;a lifetimes become &#39;static
17:42aidanhsso typeids work and all is well
17:43simulacrumAh, I guess that&#39;s true
17:43simulacrumthough, realistically, making a Triple type which is interned and &#39;static isn&#39;t all that bad of an idea anyway
17:43simulacrumnor is it that hard
17:45simulacrumAnd I think the only thing we in practice borrow is triples
17:45aidanhsthere are a few other non-triple strings to consider, so I wouldn&#39;t specialise it to be for triples right away
17:45simulacrumWhat else do we pass around?
17:46simulacrumah, I guess names and things for the crate generic steps
17:46aidanhsname of the rust books, test suite names
17:46simulacrumSure, we can have a InternedString or use a crate or something
17:46aidanhssome of those can be static, you can see that in my PR
17:46simulacrumyeah, that&#39;s true
17:48aidanhsI vaguely feel like getting rid of those lifetimes will make future extending, so whether it&#39;s making everything owned or using a leaky-interned-string crate, I&#39;m in favour
17:48aidanhs*future extending easier
17:48simulacrumYeah, I agree about making the future easier
17:49simulacrumI&#39;m not sure whether I prefer the interning approach
17:50simulacrumit seems cleaner
17:50aidanhsI prefer it to the current approach I think
17:50simulacrumaidanhs: Right, I mean interning vs leaking
17:51aidanhsah I see
17:52aidanhsinterning is probably...30 lines or so, leaking is 3 - they&#39;re easily swappable, I just suggested for the purposes of prototyping speed
17:53simulacrumaidanhs: I think I&#39;ll implement interning just for peace of mind
17:54aidanhssounds fine - the only thing that changes is the implementation of `intern_string`, call sites etc remains identical
17:55aidanhsI&#39;d probably keep it within rustbuild instead of a separate crate as well, given how compact it&#39;ll be
17:55simulacrumaidanhs: intern_string being a new method?
17:56aidanhsthat&#39;s the only interface you need
17:57simulacrumaidanhs: Well, and to find a place to put it that&#39;s in the root. I guess I could make a thread-local or lazy_static
17:58aidanhsI figured you&#39;d put a lazy_static in, seems a reasonable name to reuse!
17:59simulacrumSure. Well, we&#39;d still have the cache struct and all, I think, but maybe not -- not sure
17:59aidanhsoh I wouldn&#39;t see the cache struct going away
18:00aidanhs(the precise layout I was describing is just personal preference bikeshedding)
18:00simulacrumsure yea
18:01acrichtoshep: try --git
18:01acrichtoshep: I don&#39;t think the verson has been updated recently
18:06shepacrichto: oh, sure, but &quot;just `cargo install sccache`&quot; is what I heard
18:11acrichtoshep: you&#39;re not wrong!
18:14shepacrichto: does sccache save incremental compilation caches?
18:17acrichtoshep: not currently, no
18:37brsoni&#39;m publishing a 7/7 nightly cargobomb run and starting a beta.3 cargobomb run, and bumping to beta.4
18:42acrichtobrson: oh no your push to your beta-next branch killed bors :(
18:42acrichtowe&#39;ve also got two more backports after that one to land
18:43acrichtoalthough no one touch bors for a bit
18:43acrichtowe&#39;re like 30m away from travis/appveyor finishing
18:43acrichtoif that works I&#39;ll do the merge manually
18:44simulacrumappveyor is done already -- travis will be done in ~30 min, yeah
18:44simulacrumI&#39;m sure someone will retry without knowing
18:45simulacrumand it woke up
18:45acrichtoall we goota do is get those artifacts...
18:46acrichtobrson: noooo :(
18:47brsonyeah i screwed that up
18:47brsonthe wrong build is running now
18:51brsonacrichto: all the backports are enqued now right?
18:51acrichtobrson: afaik yes
18:51simulacrumI believe so
20:40simulacrumWell, there&#39;s one upside to having nothing go into the tree for a couple hours: perf.rlo is catching up on historic data
14 Jul 2017
No messages
Last message: 72 days and 23 hours ago