Skip to content

[per-OS Packages] fix devbox update for unversioned packages with platform/excluded-platform #1370

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

Merged
merged 1 commit into from
Aug 12, 2023

Conversation

savil
Copy link
Collaborator

@savil savil commented Aug 11, 2023

Summary

This PR fixes a slightly edge-casey workflow:

  1. User on macOS tries to devbox shell with glibcLocales in devbox.json. Note: unversioned package.
  2. Is asked to run: devbox add glibcLocales --exclude-platforms x86_64-darwin
  3. Running that, prints a notice to run devbox update.
  4. Runs devbox update.

This should work, but was previously broken for two reasons:

  1. In update.go, we were calling devbox.Add with nil for platforms and excluded_platforms.
  2. In devbox.Add, we check versionedPkg.ValidateExists() prior to converting the raw package name to the versioned-name.
    Both are now fixed.

How was it tested?

Ran the workflow described above. It's hard to write a testscript unit-test, since
some packages build may or may not build on whichever system is running the unit-test. And mocking is too complicated here.

@savil savil force-pushed the savil/config-packages-4 branch from c15183c to 5191dfb Compare August 11, 2023 22:18
@savil savil force-pushed the savil/config-packages-5 branch from 7b8aeb4 to 0b85c46 Compare August 11, 2023 22:18
@savil savil force-pushed the savil/config-packages-4 branch from 5191dfb to f69b9f3 Compare August 11, 2023 22:36
@savil savil force-pushed the savil/config-packages-5 branch from 0b85c46 to 2bd855b Compare August 11, 2023 22:36
@savil savil force-pushed the savil/config-packages-4 branch from f69b9f3 to b54fd8e Compare August 11, 2023 22:47
@savil savil force-pushed the savil/config-packages-5 branch from 2bd855b to 7a0c8d5 Compare August 11, 2023 22:47
@savil savil force-pushed the savil/config-packages-4 branch from b54fd8e to 7866592 Compare August 11, 2023 23:06
@savil savil force-pushed the savil/config-packages-5 branch from 7a0c8d5 to 44a9518 Compare August 11, 2023 23:06
savil added a commit that referenced this pull request Aug 11, 2023
…1360)

## Summary

This PR refactors `devbox.findPackagesByName` to be constructed from
`devconfig.Package`.

**Motivation**
`devbox.Update` calls `findPackageByName` and needs to call `devbox.Add`
again with the Platform and ExcludedPlatforms. (see #1370)

## How was it tested?

compiles
@savil savil force-pushed the savil/config-packages-4 branch from 7866592 to 19fb40c Compare August 11, 2023 23:09
@savil savil force-pushed the savil/config-packages-5 branch from 44a9518 to 7e8866b Compare August 11, 2023 23:09
@savil savil force-pushed the savil/config-packages-4 branch from 19fb40c to b6e2aca Compare August 11, 2023 23:15
@savil savil force-pushed the savil/config-packages-5 branch from 7e8866b to a6e4c41 Compare August 11, 2023 23:15
@savil savil force-pushed the savil/config-packages-4 branch from b6e2aca to e216027 Compare August 11, 2023 23:54
@savil savil force-pushed the savil/config-packages-5 branch from a6e4c41 to f4f1e5f Compare August 11, 2023 23:54
@savil savil force-pushed the savil/config-packages-4 branch from e216027 to e5d2bfa Compare August 12, 2023 00:03
@savil savil force-pushed the savil/config-packages-5 branch from f4f1e5f to c5d63c9 Compare August 12, 2023 00:03
@savil savil force-pushed the savil/config-packages-4 branch from e5d2bfa to 3fc324b Compare August 12, 2023 00:05
@savil savil force-pushed the savil/config-packages-5 branch from c5d63c9 to d0b96db Compare August 12, 2023 00:05
@savil savil force-pushed the savil/config-packages-4 branch from 3fc324b to 4037aa2 Compare August 12, 2023 00:29
@savil savil force-pushed the savil/config-packages-5 branch from d0b96db to ecf3d2c Compare August 12, 2023 00:29
@savil savil force-pushed the savil/config-packages-4 branch from 4037aa2 to fc76ec0 Compare August 12, 2023 00:30
@savil savil force-pushed the savil/config-packages-5 branch from ecf3d2c to 02b4ab8 Compare August 12, 2023 00:30
Base automatically changed from savil/config-packages-4 to main August 12, 2023 00:31
@savil savil force-pushed the savil/config-packages-5 branch from 02b4ab8 to 5e7b6fd Compare August 12, 2023 00:32
@savil savil merged commit 1678c13 into main Aug 12, 2023
@savil savil deleted the savil/config-packages-5 branch August 12, 2023 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants