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

Merge v2-beta changes to main #86

Merged
merged 52 commits into from
Feb 6, 2024
Merged

Merge v2-beta changes to main #86

merged 52 commits into from
Feb 6, 2024

Conversation

davidbuzinski
Copy link
Contributor

Merge the code for v2 release of our MATLAB actions. The doc was drafted for the v2-beta. I would like further doc changes/ discussions to made in a separate PR.

acampbel and others added 18 commits December 23, 2021 01:00
* adding tests to catch bugs

* updating unit tests and source code

* updating workflow to use 2nd release

* fixing update level logic

* update level deps script workaround

* testing no-op on existing install

* add output

* Update bat.yml

* Update bat.yml

* Update bat.yml

* Update src/matlab.ts

Co-authored-by: Mark Cafaro <[email protected]>

* Update matlab.Release and removed usExisting from destination

* updated release info to fix inconsistency

* changed output name to matlabroot

* fixng pipeline syntax

* missing colon in yml

* updating naming and fixing missing simulink in bat workflow

* changing some variable names for clarity

Co-authored-by: Mark Cafaro <[email protected]>
* added cache-save and updated build. TODO: cache-restore

* added cache restore

* fixing release type conversion and bool type conversion

* add unit tests

* fixing path and cache hit issues

* add architecture to cache key and fix description

* addressed review feedback

* updated MatlabPath state 'type'

* updated missed copyrights

* changing back latest URL

* update cache input description
* no more unzip mpm

* test no windows

* update url

* try with .exe extension?

* fail slow

* update url

* try cacheFile to fix file location

* take out bad secrets

* undo bad string interp

* trying fix linux mac

* use runcommand

* use runcommand

* try to specify directory

* error if no runner temp found
Don't add the sudo prefix if sudo is not on the path
* initial commit

* npm audit fix

* add batch to tc

* chmod matlab-batch

* rearranging chmod

* still wrangling chmod

* remove pct

* remove PCT from defaults

* update tests

* cache fix for support packages

* make sure support packages path is in cachePaths if defined

* add log for supportPackagesPath

* capitalize release dir

* had wrong default dir

* testing windows thing

* remove log. use tmpdir for windows

* applying mpm fixes for mac & windows

* initial commit

* npm audit fix

* add batch to tc

* chmod matlab-batch

* rearranging chmod

* still wrangling chmod

* remove pct

* remove PCT from defaults

* update tests

* cache fix for support packages

* make sure support packages path is in cachePaths if defined

* add log for supportPackagesPath

* capitalize release dir

* had wrong default dir

* testing windows thing

* remove log. use tmpdir for windows

* applying mpm fixes for mac & windows

* Merge windows optimization to v2-rc0

* improve os specific cache dir implemenation

* try to link after install

* symlink wrong direction?

* update toolcache impl

* forgot .keep

* forgot dist

* fix typo

* debug

* more debug idk

* forgot recursive

* seeing whatever is going on with .complete file

* add tests for windows toolcache optimization

* prep for merge

* one more update before merge

* Update bat.yml to v2-rc0 for build until release

* address sam's feedback

* address mark's feedback

* fix error message

* switch statement sugar

Co-authored-by: Mark Cafaro <[email protected]>

* Accept any number of spaces in product list

Co-authored-by: Mark Cafaro <[email protected]>

---------

Co-authored-by: sameagen <[email protected]>
Co-authored-by: Mark Cafaro <[email protected]>
* init update

* update bat.yml

* update bat.yml
README.md Outdated
@@ -1,11 +1,11 @@
# Action for Setting Up MATLAB on GitHub-Hosted Runner

Before you run MATLAB&reg; code and Simulink&reg; models on a [GitHub&reg;-hosted](https://docs.github.com/en/free-pro-team@latest/actions/reference/specifications-for-github-hosted-runners) runner, first use the [Setup MATLAB](#set-up-matlab) action. The action sets up the specified MATLAB release on a Linux&reg; virtual machine. If you do not specify a release, the action sets up the latest release of MATLAB.
Before you run MATLAB&reg; code and Simulink&reg; models on a [GitHub&reg;-hosted](https://docs.github.com/en/free-pro-team@latest/actions/reference/specifications-for-github-hosted-runners) runner, first use the [Setup MATLAB](#set-up-matlab) action. The action sets up the specified MATLAB release on a Linux&reg; or Windows&reg; virtual machine. If you do not specify a release, the action sets up the latest release of MATLAB.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should say macOS too but it sounds like @mw-hrastega is going to handle the doc updates later?

@@ -9,6 +9,22 @@ inputs:
MATLAB release to set up (R2020a or later)
required: false
default: latest
products:
description: >-
Array of products to install
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Array" sounds like this should be specified as a YAML array. Perhaps "Products to install, specified as a list of product names separated by spaces"?

uses: matlab-actions/run-command@v2-rc0
with:
command: "${{ matrix.check-simulink }}"
- name: Check NoOp on 2nd install
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need some sort of actual noop check in the future?

Copy link
Contributor Author

@davidbuzinski davidbuzinski Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that could be nice. Not immediately clear to me what that would look like

src/cache-save.unit.test.ts Outdated Show resolved Hide resolved

async function windowsHostedToolpath(release: Release): Promise<string> {
// bail early if not on a github hosted runner
if (process.env['RUNNER_ENVIRONMENT'] !== 'github-hosted' && process.env['AGENT_ISSELFHOSTED'] === '1') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does it come up that a github-hosted runner has AGENT_ISSELFHOSTED set to true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know because it's not documented. This is how github does the checks in their internal actions. Because it's not documented, I felt most comfortable doing what they do.


const exitCode = await exec.exec(`chmod +x "${mpm}"`);
if (exitCode !== 0) {
return Promise.reject(Error("Unable to set up mpm."));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More descriptive error message here to match the Circle error message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure which Circle error message this refers to. I don't think Circle handles this error with any error message. This is the error that will print if chmod +x mpm fails

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could say something like "Unable to make mpm executable" or "Unable to add executable permission to mpm" (@mw-hrastega can wordsmith this).

src/script.ts Outdated Show resolved Hide resolved
src/script.ts Outdated Show resolved Hide resolved
src/script.unit.test.ts Outdated Show resolved Hide resolved
davidbuzinski and others added 2 commits February 5, 2024 08:51
* Update README.md

replacing v1 with v2

* removing text on platform restrictions

* replacing a verb

* updating the action description

* replacing 20b with 21a

* updating products description

* updating the cache description

* adding a placeholder section

* adding missing trademark symbol

* update the first example description

* removing a code block

* updating the last example description

* addressing review feedback

* Fix a typo.

Co-authored-by: Mark Cafaro <[email protected]>

---------

Co-authored-by: Mark Cafaro <[email protected]>
@davidbuzinski davidbuzinski merged commit 1d65426 into main Feb 6, 2024
4 of 5 checks passed
@davidbuzinski davidbuzinski deleted the mpm branch February 6, 2024 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants