Skip to content

Commit

Permalink
refactor: inject fetcher instead of using global (AztecProtocol/aztec…
Browse files Browse the repository at this point in the history
…-packages#5502)

This PR refactors the GithubDependencyResolver to take a fetch
implementation instead of always using the global default
  • Loading branch information
AztecBot committed Mar 28, 2024
1 parent 0bc18c4 commit a62e972
Show file tree
Hide file tree
Showing 12 changed files with 20 additions and 54 deletions.
2 changes: 1 addition & 1 deletion .aztec-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
a18288d9b8f3057b9e79362d922da656dacf22a9
a066544cda095adda0c1dc7918c64ecad8656b91
14 changes: 1 addition & 13 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,6 @@ jobs:
- name: Install Yarn dependencies
uses: ./.github/actions/setup

- name: Install wasm-bindgen-cli
uses: taiki-e/install-action@v2
with:
tool: wasm-bindgen-cli@0.2.86

- name: Install wasm-opt
run: |
npm i wasm-opt -g
- name: Query new noir version
id: noir-version
run: |
Expand Down Expand Up @@ -116,20 +107,17 @@ jobs:
if: ${{ always() }}

needs:
- release-please
- update-acvm-workspace-package-versions
- update-docs

env:
# We treat any skipped or failing jobs as a failure for the workflow as a whole.
FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }}
FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }

steps:
- name: Add warning to sticky comment
uses: marocchino/sticky-pull-request-comment@v2
with:
# We need to specify the PR on which to make the comment as workflow is triggered by push.
number: ${{ fromJSON(needs.release-please.outputs.release-pr).number }}
# delete the comment in case failures have been fixed
delete: ${{ !env.FAIL }}
message: "The release workflow has not completed successfully. Releasing now will result in a broken release"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,9 @@ impl<'a> ValueMerger<'a> {
/// such as with dynamic indexing of non-homogenous slices.
fn make_slice_dummy_data(&mut self, typ: &Type) -> ValueId {
match typ {
Type::Numeric(numeric_type) => {
Type::Numeric(_) => {
let zero = FieldElement::zero();
self.dfg.make_constant(zero, Type::Numeric(*numeric_type))
self.dfg.make_constant(zero, Type::field())
}
Type::Array(element_types, len) => {
let mut array = im::Vector::new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ import { LogData } from '../../utils';
*/
export class GithubDependencyResolver implements DependencyResolver {
#fm: FileManager;
#fetch: typeof fetch;
#log;

constructor(fm: FileManager) {
constructor(fm: FileManager, fetcher: typeof fetch) {
this.#fm = fm;
this.#fetch = fetcher;
this.#log = (msg: string, _data?: LogData) => {
console.log(msg);
};
Expand Down Expand Up @@ -56,7 +58,7 @@ export class GithubDependencyResolver implements DependencyResolver {
return localArchivePath;
}

const response = await fetch(url, {
const response = await this.#fetch(url, {
method: 'GET',
});

Expand Down
3 changes: 2 additions & 1 deletion compiler/wasm/src/noir/noir-wasm-compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ export class NoirWasmCompiler {
const dependencyManager = new DependencyManager(
[
new LocalDependencyResolver(fileManager),
new GithubCodeArchiveDependencyResolver(fileManager),
// use node's global fetch
new GithubCodeArchiveDependencyResolver(fileManager, fetch),
// TODO support actual Git repositories
],
noirPackage,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('GithubDependencyResolver', () => {
let fetchStub: SinonStub | undefined;

beforeEach(() => {
fetchStub = Sinon.stub(globalThis, 'fetch');
fetchStub = Sinon.stub();
fm = createMemFSFileManager(createFsFromVolume(new Volume()), '/');

libDependency = {
Expand All @@ -50,16 +50,12 @@ describe('GithubDependencyResolver', () => {
},
});

resolver = new GithubDependencyResolver(fm);
resolver = new GithubDependencyResolver(fm, fetchStub);

// cut off outside access
fetchStub.onCall(0).throws(new Error());
});

afterEach(() => {
fetchStub?.restore();
});

it("returns null if it can't resolve a dependency", async () => {
const dep = await resolver.resolveDependency(pkg, {
path: '/lib-c',
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

2 changes: 1 addition & 1 deletion tooling/bb_abstraction_leaks/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use const_format::formatcp;

const USERNAME: &str = "AztecProtocol";
const REPO: &str = "aztec-packages";
const VERSION: &str = "0.32.0";
const VERSION: &str = "0.30.1";
const TAG: &str = formatcp!("aztec-packages-v{}", VERSION);

const API_URL: &str =
Expand Down
2 changes: 1 addition & 1 deletion tooling/noir_js_backend_barretenberg/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore --max-warnings 0"
},
"dependencies": {
"@aztec/bb.js": "0.32.0",
"@aztec/bb.js": "portal:../../../../barretenberg/ts",
"@noir-lang/types": "workspace:*",
"fflate": "^0.8.0"
},
Expand Down
13 changes: 6 additions & 7 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -221,19 +221,18 @@ __metadata:
languageName: node
linkType: hard

"@aztec/bb.js@npm:0.32.0":
version: 0.32.0
resolution: "@aztec/bb.js@npm:0.32.0"
"@aztec/bb.js@portal:../../../../barretenberg/ts::locator=%40noir-lang%2Fbackend_barretenberg%40workspace%3Atooling%2Fnoir_js_backend_barretenberg":
version: 0.0.0-use.local
resolution: "@aztec/bb.js@portal:../../../../barretenberg/ts::locator=%40noir-lang%2Fbackend_barretenberg%40workspace%3Atooling%2Fnoir_js_backend_barretenberg"
dependencies:
comlink: ^4.4.1
commander: ^10.0.1
debug: ^4.3.4
tslib: ^2.4.0
bin:
bb.js: dest/node/main.js
checksum: 0919957e141ae0a65cfab961dce122fa06de628a10b7cb661d31d8ed4793ce80980fcf315620ceffffa45581db941bad43c392f4b2aa9becaaf7d2faaba01ffc
bb.js: ./dest/node/main.js
languageName: node
linkType: hard
linkType: soft

"@babel/code-frame@npm:^7.0.0, @babel/code-frame@npm:^7.10.4, @babel/code-frame@npm:^7.12.11, @babel/code-frame@npm:^7.16.0, @babel/code-frame@npm:^7.22.13, @babel/code-frame@npm:^7.23.5, @babel/code-frame@npm:^7.8.3":
version: 7.23.5
Expand Down Expand Up @@ -4396,7 +4395,7 @@ __metadata:
version: 0.0.0-use.local
resolution: "@noir-lang/backend_barretenberg@workspace:tooling/noir_js_backend_barretenberg"
dependencies:
"@aztec/bb.js": 0.32.0
"@aztec/bb.js": "portal:../../../../barretenberg/ts"
"@noir-lang/types": "workspace:*"
"@types/node": ^20.6.2
"@types/prettier": ^3
Expand Down

0 comments on commit a62e972

Please sign in to comment.