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.

Sign up for GitHub

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

Jump to bottom

Fix install fasm directly from GitHub #62

Merged
merged 5 commits into from
Feb 20, 2021
Merged

Fix install fasm directly from GitHub #62

merged 5 commits into from
Feb 20, 2021

Conversation

mithro
Copy link
Contributor

@mithro mithro commented Feb 19, 2021 •
edited

This pull request;

  1. Adds a workflow which makes sure that installing from GitHub works. It was able to replicate the issue described in Latest fasm setup.py no longer works as a git+https URL #60 -- see https://github.com/mithro/fasm/runs/1938179073?check_suite_focus=true
  2. Fixes Latest fasm setup.py no longer works as a git+https URL #60 by calling update_version.py in setup.py if the version is currently UNKNOWN.
  3. Reworks update_version.py to not depend on the packaging module.
  4. Fixes Broken installation due to probable circular import #57 by updating the tool main location.
  5. Improves the warning message when the antlr parser is not found.

@mithro
ci: Adding workflow to check installing from GitHub works.
698f520 
Signed-off-by: Tim 'mithro' Ansell <me@mith.ro>
@mithro
Run update_version.py script if version is 'UNKNOWN'.
6137b3b 
Fixes chipsalliance#60

Signed-off-by: Tim 'mithro' Ansell <me@mith.ro>
@mithro mithro requested a review from litghost February 19, 2021 21:08
This was referenced Feb 19, 2021
Closed
Closed
@mithro
Don't use packaging module in update_version.py
2629fd2 
Signed-off-by: Tim 'mithro' Ansell <me@mith.ro>
@mithro
Correct path to fasm tool in setup.py
037bd74 
Fixes chipsalliance#57.

Signed-off-by: Tim 'mithro' Ansell <me@mith.ro>
@mithro
Improve the warning message when falling back to the textX.
6bdbb14 
Signed-off-by: Tim 'mithro' Ansell <me@mith.ro>
mithro
mithro commented Feb 19, 2021
View reviewed changes
.github/workflows/check-install.yml
runs-on: ${{ matrix.os }}

steps:
- name: Install dependencies (Ubuntu)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really should test that install succeeds when these dependencies are not found.

litghost
litghost approved these changes Feb 19, 2021
View reviewed changes
Copy link
Collaborator

@litghost litghost left a comment

Choose a reason for hiding this comment

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

LGTM, merge if green

@litghost litghost merged commit af39a4f into chipsalliance:master Feb 20, 2021
@mithro mithro deleted the fix-github-install branch February 20, 2021 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@litghost litghost litghost approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Latest fasm setup.py no longer works as a git+https URL Broken installation due to probable circular import
2 participants
@mithro @litghost