Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Divergence when bundling yarn install --production #919

Closed
michaelfig opened this issue Oct 16, 2021 · 6 comments
Closed

Divergence when bundling yarn install --production #919

michaelfig opened this issue Oct 16, 2021 · 6 comments
Assignees
Labels
bug Something isn't working design endo

Comments

@michaelfig
Copy link
Member

I have checked that dev mode is disabled, but https://github.com/Agoric/agoric-sdk/tree/mfig-bundle-divergence/dev-vs-prod-bundle.sh reveals a difference between bundling in the two different yarn install situations.

TODO: More details

@kriskowal
Copy link
Member

For reference, these are the differences in package layout between dev and prod: https://gist.github.com/kriskowal/dc51be6c08234a579f4eef33fd4a4bc8

The production steps:

yarn --frozen-lockfile
find . -name package.json | while read pkg; do jq -r --arg pkg "$(dirname $pkg)" '"\(.name)@\(.version) \($pkg)"' "$pkg"; done | sort > dev.list
yarn --frozen-lockfile --production
find . -name package.json | while read pkg; do jq -r --arg pkg "$(dirname $pkg)" '"\(.name)@\(.version) \($pkg)"' "$pkg"; done | sort > prod.list
diff dev.list prod.list

@kriskowal
Copy link
Member

Also for reference, this is an example of the differences between dev and prod for one bundle in a zip file:

https://gist.github.com/kriskowal/f927e01a0cb9d9ab0b543543b39057f0

@kriskowal
Copy link
Member

Arranging for bundles to be the same regardless of whether they are generated from files as laid out by yarn for development or production may only be possible by much more rigorous means.

Yarn and Npm are certainly allowed to put multiple copies of the same package [name, version] in the node_modules tree, but I think we can count on Yarn at least to use symbolic links to point to a single hard copy. Proceeding on that assumption.

I can remove the order numbers from the compartment names and merge any packages that have the same name and version. It should not be necessary to incorporate a consistent hash of the package, which is good because that would be expensive to obtain (the compartment mapper doesn’t have to read every file in a package, but a hasher would). We should at least verify that there is only one canonical location per name@version in a compartment map.

I’m no longer convinced that the solution to this problem is shaking out the unused edges in the compartment map.

@michaelfig
Copy link
Member Author

I can remove the order numbers from the compartment names and merge any packages that have the same name and version.

Is this what you are proposing as the solution? That would be great!

Don't let me nerd-snipe you with this problem, though. Only address what seems useful and addressable.

@kriskowal
Copy link
Member

It’s an idea for a solution. I’m not sure that it will work.

kriskowal added a commit that referenced this issue Nov 17, 2021
Please see the detailed comments for the rationale for this change. The result should be that archive hashes are insensitive to whether the package manager installed development or production dependencies.

Fixes #919
@Tartuffo Tartuffo added the MN-1 label Feb 2, 2022
@Tartuffo
Copy link

Tartuffo commented Feb 3, 2022

@michaelfig @kriskowal This does not have an area label that is covered by our weekly tech / planning meetings. Can you assign the proper label? We cover: agd, agoric-cosmos, amm, core economy, cosmic-swingset, endo, getrun, governance, installation-bundling, metering, run-protocol, staking, swingset, swingset-runner, token economy, ui, wallet, zoe, zoe contract

@michaelfig michaelfig added the endo label Feb 3, 2022
@Tartuffo Tartuffo added design and removed MN-1 labels Feb 7, 2022
kriskowal added a commit that referenced this issue Apr 26, 2022
Previously, compartment names in an archive always included a sequence
number, which allowed for the possibility of two packages with the same
name and version.
With this change, a sequence number is replaced with a duplicate
number only when there are multiple packages with the same number.

Please see the detailed comments for the rationale for this change.
The result should be that archive hashes are much less sensitive
to differences in the layout of the dependency graph.

Fixes #919
kriskowal added a commit that referenced this issue Apr 26, 2022
Previously, compartment names in an archive always included a sequence
number, which allowed for the possibility of two packages with the same
name and version.
With this change, a sequence number is replaced with a duplicate
number only when there are multiple packages with the same number.

Please see the detailed comments for the rationale for this change.
The result should be that archive hashes are much less sensitive
to differences in the layout of the dependency graph.

Fixes #919
kriskowal added a commit that referenced this issue Apr 26, 2022
Previously, compartment names in an archive always included a sequence
number, which allowed for the possibility of two packages with the same
name and version.
With this change, a sequence number is replaced with a duplicate
number only when there are multiple packages with the same number.

Please see the detailed comments for the rationale for this change.
The result should be that archive hashes are much less sensitive
to differences in the layout of the dependency graph.

Fixes #919
kriskowal added a commit that referenced this issue Apr 26, 2022
Previously, compartment names in an archive always included a sequence
number, which allowed for the possibility of two packages with the same
name and version.
With this change, a sequence number is replaced with a duplicate
number only when there are multiple packages with the same number.

Please see the detailed comments for the rationale for this change.
The result should be that archive hashes are much less sensitive
to differences in the layout of the dependency graph.

Fixes #919
kriskowal added a commit that referenced this issue Apr 26, 2022
Previously, compartment names in an archive always included a sequence
number, which allowed for the possibility of two packages with the same
name and version.
With this change, a sequence number is replaced with a duplicate
number only when there are multiple packages with the same number.

Please see the detailed comments for the rationale for this change.
The result should be that archive hashes are much less sensitive
to differences in the layout of the dependency graph.

Fixes #919
naugtur pushed a commit that referenced this issue May 16, 2022
Previously, compartment names in an archive always included a sequence
number, which allowed for the possibility of two packages with the same
name and version.
With this change, a sequence number is replaced with a duplicate
number only when there are multiple packages with the same number.

Please see the detailed comments for the rationale for this change.
The result should be that archive hashes are much less sensitive
to differences in the layout of the dependency graph.

Fixes #919
naugtur pushed a commit that referenced this issue May 16, 2022
Previously, compartment names in an archive always included a sequence
number, which allowed for the possibility of two packages with the same
name and version.
With this change, a sequence number is replaced with a duplicate
number only when there are multiple packages with the same number.

Please see the detailed comments for the rationale for this change.
The result should be that archive hashes are much less sensitive
to differences in the layout of the dependency graph.

Fixes #919
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working design endo
Projects
None yet
Development

No branches or pull requests

3 participants