Skip to content

Commit

Permalink
fix: hangar snapshots have machine-dependent output (#3777)
Browse files Browse the repository at this point in the history
In #3743 I introduced some code to avoid name collisions in our code generation by hashing file paths. It seems this was adequate for many cases, but it caused problems for contribution since our `hangar` E2E tests currently compile most Wing programs using absolute paths. That means in these scenarios the hashes will be machine dependent. According to @MarkMcCulloh these tests are needed since there were bugs with absolute paths in the past. To that end, this PR changes the name deduplication strategy to be based around incrementing numbers.

## Checklist

- [ ] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [ ] Description explains motivation and solution
- [ ] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
  • Loading branch information
Chriscbr authored Aug 10, 2023
1 parent 8df5d0e commit e97e3bb
Show file tree
Hide file tree
Showing 272 changed files with 1,135 additions and 1,144 deletions.
21 changes: 0 additions & 21 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion libs/wingc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ const_format = "0.2.30"
duplicate = "1.0.0"
strum = { version = "0.24", features = ["derive"] }
petgraph = "0.6.3"
sha1 = "0.10.5"

[lib]
crate-type = ["rlib", "cdylib"]
Expand Down
33 changes: 23 additions & 10 deletions libs/wingc/src/jsify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use aho_corasick::AhoCorasick;
use const_format::formatcp;
use indexmap::{IndexMap, IndexSet};
use itertools::Itertools;
use sha1::{Digest, Sha1};

use std::{
borrow::Borrow,
Expand Down Expand Up @@ -59,6 +58,12 @@ pub struct JSifier<'a> {
pub output_files: RefCell<Files>,
/// Counter for generating unique preflight file names.
preflight_file_counter: RefCell<usize>,

/// Counter for generating unique inflight file names.
inflight_file_counter: RefCell<usize>,
/// Map from source file IDs to safe counters.
inflight_file_map: RefCell<IndexMap<String, usize>>,

/// Map from source file paths to the JS file names they are emitted to.
/// e.g. "bucket.w" -> "preflight.bucket-1.js"
preflight_file_map: RefCell<IndexMap<PathBuf, String>>,
Expand Down Expand Up @@ -90,6 +95,8 @@ impl<'a> JSifier<'a> {
source_files,
entrypoint_file_path,
absolute_project_root,
inflight_file_counter: RefCell::new(0),
inflight_file_map: RefCell::new(IndexMap::new()),
preflight_file_counter: RefCell::new(0),
preflight_file_map: RefCell::new(IndexMap::new()),
output_files: RefCell::new(output_files),
Expand Down Expand Up @@ -1051,7 +1058,7 @@ impl<'a> JSifier<'a> {
}

fn jsify_to_inflight_type_method(&self, class: &AstClass, ctx: &JSifyContext) -> CodeMaker {
let client_path = inflight_filename(class);
let client_path = self.inflight_filename(class);

let mut code = CodeMaker::default();

Expand Down Expand Up @@ -1170,7 +1177,7 @@ impl<'a> JSifier<'a> {
match self
.output_files
.borrow_mut()
.add_file(inflight_filename(class), code.to_string())
.add_file(self.inflight_filename(class), code.to_string())
{
Ok(()) => {}
Err(err) => report_diagnostic(err.into()),
Expand Down Expand Up @@ -1287,6 +1294,19 @@ impl<'a> JSifier<'a> {
bind_method.close("}");
bind_method
}

fn inflight_filename(&self, class: &AstClass) -> String {
let mut file_map = self.inflight_file_map.borrow_mut();
let id: usize = if file_map.contains_key(&class.name.span.file_id) {
file_map[&class.name.span.file_id]
} else {
let mut id = self.inflight_file_counter.borrow_mut();
*id += 1;
file_map.insert(class.name.span.file_id.clone(), *id);
*id
};
format!("./inflight.{}-{}.js", class.name.name, id)
}
}

fn get_public_symbols(scope: &Scope) -> Vec<Symbol> {
Expand Down Expand Up @@ -1325,13 +1345,6 @@ fn get_public_symbols(scope: &Scope) -> Vec<Symbol> {
symbols
}

fn inflight_filename(class: &AstClass) -> String {
let mut hasher = Sha1::new();
hasher.update(&class.name.span.file_id);
let hash = format!("{:x}", hasher.finalize()); // convert to hex
format!("./inflight.{}-{}.js", class.name.name, &hash[hash.len() - 8..])
}

fn lookup_span(span: &WingSpan, files: &Files) -> String {
let source = files
.get_file(&span.file_id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ source: libs/wingc/src/jsify/tests.rs
```

## inflight.$Closure1-220cc4cd.js
## inflight.$Closure1-1.js

```js
module.exports = function({ $_y_at_0__, $x_length }) {
Expand Down Expand Up @@ -52,7 +52,7 @@ class $Root extends $stdlib.std.Resource {
}
static _toInflightType(context) {
return $stdlib.core.NodeJsCode.fromInline(`
require("./inflight.$Closure1-220cc4cd.js")({
require("./inflight.$Closure1-1.js")({
$_y_at_0__: ${context._lift((y.at(0)))},
$x_length: ${context._lift(x.length)},
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ source: libs/wingc/src/jsify/tests.rs
```

## inflight.$Closure1-220cc4cd.js
## inflight.$Closure1-1.js

```js
module.exports = function({ $s_length }) {
Expand Down Expand Up @@ -49,7 +49,7 @@ class $Root extends $stdlib.std.Resource {
}
static _toInflightType(context) {
return $stdlib.core.NodeJsCode.fromInline(`
require("./inflight.$Closure1-220cc4cd.js")({
require("./inflight.$Closure1-1.js")({
$s_length: ${context._lift(s.length)},
})
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ source: libs/wingc/src/jsify/tests.rs
```

## inflight.$Closure1-220cc4cd.js
## inflight.$Closure1-1.js

```js
module.exports = function({ $_s___hello___length }) {
Expand Down Expand Up @@ -49,7 +49,7 @@ class $Root extends $stdlib.std.Resource {
}
static _toInflightType(context) {
return $stdlib.core.NodeJsCode.fromInline(`
require("./inflight.$Closure1-220cc4cd.js")({
require("./inflight.$Closure1-1.js")({
$_s___hello___length: ${context._lift((s)["hello"].length)},
})
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ source: libs/wingc/src/jsify/tests.rs
```

## inflight.Base-220cc4cd.js
## inflight.Base-1.js

```js
module.exports = function({ $x }) {
Expand All @@ -36,7 +36,7 @@ module.exports = function({ $x }) {
}
```

## inflight.Derived-220cc4cd.js
## inflight.Derived-1.js

```js
module.exports = function({ $Base }) {
Expand Down Expand Up @@ -69,7 +69,7 @@ class $Root extends $stdlib.std.Resource {
}
static _toInflightType(context) {
return $stdlib.core.NodeJsCode.fromInline(`
require("./inflight.Base-220cc4cd.js")({
require("./inflight.Base-1.js")({
$x: ${context._lift(x)},
})
`);
Expand Down Expand Up @@ -99,7 +99,7 @@ class $Root extends $stdlib.std.Resource {
}
static _toInflightType(context) {
return $stdlib.core.NodeJsCode.fromInline(`
require("./inflight.Derived-220cc4cd.js")({
require("./inflight.Derived-1.js")({
$Base: ${context._lift(Base)},
})
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ source: libs/wingc/src/jsify/tests.rs
```

## inflight.Base-220cc4cd.js
## inflight.Base-1.js

```js
module.exports = function({ }) {
Expand All @@ -33,7 +33,7 @@ module.exports = function({ }) {
}
```

## inflight.Derived-220cc4cd.js
## inflight.Derived-1.js

```js
module.exports = function({ $Base }) {
Expand Down Expand Up @@ -66,7 +66,7 @@ class $Root extends $stdlib.std.Resource {
}
static _toInflightType(context) {
return $stdlib.core.NodeJsCode.fromInline(`
require("./inflight.Base-220cc4cd.js")({
require("./inflight.Base-1.js")({
})
`);
}
Expand All @@ -92,7 +92,7 @@ class $Root extends $stdlib.std.Resource {
}
static _toInflightType(context) {
return $stdlib.core.NodeJsCode.fromInline(`
require("./inflight.Derived-220cc4cd.js")({
require("./inflight.Derived-1.js")({
$Base: ${context._lift(Base)},
})
`);
Expand Down
8 changes: 4 additions & 4 deletions libs/wingc/src/jsify/snapshots/base_class_lift_indirect.snap
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ source: libs/wingc/src/jsify/tests.rs
```

## inflight.Base-220cc4cd.js
## inflight.Base-1.js

```js
module.exports = function({ }) {
Expand All @@ -44,7 +44,7 @@ module.exports = function({ }) {
}
```

## inflight.Derived-220cc4cd.js
## inflight.Derived-1.js

```js
module.exports = function({ $Base }) {
Expand Down Expand Up @@ -79,7 +79,7 @@ class $Root extends $stdlib.std.Resource {
}
static _toInflightType(context) {
return $stdlib.core.NodeJsCode.fromInline(`
require("./inflight.Base-220cc4cd.js")({
require("./inflight.Base-1.js")({
})
`);
}
Expand Down Expand Up @@ -112,7 +112,7 @@ class $Root extends $stdlib.std.Resource {
}
static _toInflightType(context) {
return $stdlib.core.NodeJsCode.fromInline(`
require("./inflight.Derived-220cc4cd.js")({
require("./inflight.Derived-1.js")({
$Base: ${context._lift(Base)},
})
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ source: libs/wingc/src/jsify/tests.rs
```

## inflight.Base-220cc4cd.js
## inflight.Base-1.js

```js
module.exports = function({ }) {
Expand All @@ -41,7 +41,7 @@ module.exports = function({ }) {
}
```

## inflight.Derived-220cc4cd.js
## inflight.Derived-1.js

```js
module.exports = function({ $Base }) {
Expand Down Expand Up @@ -78,7 +78,7 @@ class $Root extends $stdlib.std.Resource {
}
static _toInflightType(context) {
return $stdlib.core.NodeJsCode.fromInline(`
require("./inflight.Base-220cc4cd.js")({
require("./inflight.Base-1.js")({
})
`);
}
Expand All @@ -101,7 +101,7 @@ class $Root extends $stdlib.std.Resource {
}
static _toInflightType(context) {
return $stdlib.core.NodeJsCode.fromInline(`
require("./inflight.Derived-220cc4cd.js")({
require("./inflight.Derived-1.js")({
$Base: ${context._lift(Base)},
})
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ source: libs/wingc/src/jsify/tests.rs
```

## inflight.Base-220cc4cd.js
## inflight.Base-1.js

```js
module.exports = function({ }) {
Expand All @@ -38,7 +38,7 @@ module.exports = function({ }) {
}
```

## inflight.Derived-220cc4cd.js
## inflight.Derived-1.js

```js
module.exports = function({ $Base }) {
Expand Down Expand Up @@ -69,7 +69,7 @@ class $Root extends $stdlib.std.Resource {
}
static _toInflightType(context) {
return $stdlib.core.NodeJsCode.fromInline(`
require("./inflight.Base-220cc4cd.js")({
require("./inflight.Base-1.js")({
})
`);
}
Expand Down Expand Up @@ -97,7 +97,7 @@ class $Root extends $stdlib.std.Resource {
}
static _toInflightType(context) {
return $stdlib.core.NodeJsCode.fromInline(`
require("./inflight.Derived-220cc4cd.js")({
require("./inflight.Derived-1.js")({
$Base: ${context._lift(Base)},
})
`);
Expand Down
Loading

0 comments on commit e97e3bb

Please sign in to comment.