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

restore CI pipeline test for list-* commands #828

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ddalcino
Copy link
Contributor

@ddalcino ddalcino commented Oct 6, 2024

Restore CI pipeline test for list-* commands.

See bdf8b4b#r147631586

I'm slightly concerned about using strings like list-* in Bash because of potential issues with wildcard character expansion. As long as these strings are quoted, I think they should be fine though.

@ddalcino
Copy link
Contributor Author

ddalcino commented Oct 6, 2024

There's one failing run here (https://github.com/miurahr/aqtinstall/pull/828/checks?check_run_id=31143733858) that reproduces #817.

@Edgars-Cirulis
Copy link

Anything we can do about it?

Copy link
Owner

@miurahr miurahr left a comment

Choose a reason for hiding this comment

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

It is better to avoid an asterisk. Is there any other better name?

ci/generate_azure_pipelines_matrices.py Outdated Show resolved Hide resolved
ci/steps.yml Outdated Show resolved Hide resolved
Avoid the use of `*` in subcommand names

Co-authored-by: Hiroshi Miura <[email protected]>
@ddalcino
Copy link
Contributor Author

It is better to avoid an asterisk. Is there any other better name?

Agreed; list-commands is better.

@miurahr
Copy link
Owner

miurahr commented Oct 15, 2024

aqt list-qt linux android failed when visit qt6_7_3_backup folder.

In metadata.py: 779

        def folder_to_version_extension(folder: str) -> Tuple[Optional[Version], str]:
            components = folder.split("_", maxsplit=2)
            ext = "" if len(components) < 3 else components[2]
            ver = "" if len(components) < 2 else components[1]
            return (
                get_semantic_version(qt_ver=ver, is_preview="preview" in ext),
                ext,
            )

components become ['qt6', '7', '3_backup'] and ver become 7

@ddalcino
Copy link
Contributor Author

aqt list-qt linux android failed when visit qt6_7_3_backup folder.

Good, it should be failing there because no one has fixed #817 yet.

@Edgars-Cirulis
Copy link

aqt list-qt linux android failed when visit qt6_7_3_backup folder.

In metadata.py: 779

        def folder_to_version_extension(folder: str) -> Tuple[Optional[Version], str]:
            components = folder.split("_", maxsplit=2)
            ext = "" if len(components) < 3 else components[2]
            ver = "" if len(components) < 2 else components[1]
            return (
                get_semantic_version(qt_ver=ver, is_preview="preview" in ext),
                ext,
            )

components become ['qt6', '7', '3_backup'] and ver become 7

I barely used python but I can try to fix by logic

@ddalcino
Copy link
Contributor Author

No, @Edgars-Cirulis, it's a lot more complicated than just this function. List-qt needs to look in two separate places for versions of qt for android now; this will be a major architectural change if we want it to report accurate results. You can try it if you want, but it will be a bigger job than you think.

@Edgars-Cirulis
Copy link

No, @Edgars-Cirulis, it's a lot more complicated than just this function. List-qt needs to look in two separate places for versions of qt for android now; this will be a major architectural change if we want it to report accurate results. You can try it if you want, but it will be a bigger job than you think.

Could you describe the "two separate places" you're referring to? Thanks!

I initially thought the issue was about handling folder names with multiple underscores in folder_to_version_extension, but your message made me.. theres something else.

@miurahr
Copy link
Owner

miurahr commented Oct 16, 2024

@Edgars-Cirulis could you read the discussion thread here ?
#824 (comment)

For Android, there are several URLs for versions.
#824 (reply in thread)

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.

3 participants