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

Resolve dependencies and rebuild, squash warnings #79

Merged
merged 8 commits into from Oct 6, 2020

Conversation

bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Oct 6, 2020

This resolves the dependencies updates suggested by #77 and #69, but also runs the build so might be simpler than taking those.

This should also fix #78 (once released).

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 6, 2020

The upgrade reveals some new warnings:

https://github.com/conda-incubator/setup-miniconda/actions/runs/291485813

Please run using "bash" or "sh", but not "." or "source"\n/home/runner/work/_temp/47b7cea6-d0a4-4996-8834-c36d4302b2f5: line 13: return: can only `return' from a function or sourced script

Please run using "bash" or "sh", but not "." or "source"\n

as well as of course the conda upgrade... will take a look into squashing them...

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 6, 2020

looks like that's just some juiced-up warnings... the only thing i've seen is to redirect the output, e.g. bash miniconda.sh > /tmp/installer-output. The installer output is not very interesting, anyway, so maybe we only show it in case of error?

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 6, 2020

ah, i see execute has an error swallowing mechanism, will add to that...

Copy link
Member

@goanpeca goanpeca left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @bollwyvl

@goanpeca
Copy link
Member

goanpeca commented Oct 6, 2020

Are we good to merge after tests pass?

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 6, 2020

I'd like to get all the errors we know about today squashed, since that's what people see... so some approach from #58 (comment) would be good...

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 6, 2020

Ok, I have the ones from #79 (comment) squared away.

We just need to pick (some) approaches for the conda update notify, happy to do them here!

@goanpeca
Copy link
Member

goanpeca commented Oct 6, 2020

Ping me or merge when ready @bollwyvl the overall approach looks good, so thanks again :-)

@bollwyvl bollwyvl changed the title Resolve dependencies and rebuild Resolve dependencies and rebuild, squash warnings Oct 6, 2020
@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 6, 2020

@goanpeca Ok, the bootstrap condarc does the trick. It gets overwritten if the user specifies a conda file, which is fine, i think, at least they will know it can be used, and can control. I'd say this is ready to go!

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 6, 2020

the only warning this action doesn't emit is the only_use_tar_bz2 on windows with its old conda... but then, I'm not super confident on caching on windows at present, anyways...

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 6, 2020

Pushed for #79 (comment) (Other Shells should have no warnings)

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 6, 2020

Yep, that did it: we're down to just first-party warnings, not planning to do anything else here...

@bollwyvl bollwyvl requested a review from goanpeca October 6, 2020 15:11
@goanpeca goanpeca merged commit 270ae70 into conda-incubator:master Oct 6, 2020
@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 6, 2020 via email

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 6, 2020

So can this already be tested out, or do we need a release?

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 6, 2020

Trying out @master, it looks like the explicit stuff was never tested on windows, and therefore doesn't work... sigh.

https://github.com/krassowski/jupyterlab-lsp/pull/348/checks?check_run_id=1216272360

nicolas-entourage added a commit to nicolas-entourage/aws-ecr-deploy that referenced this pull request Feb 1, 2021
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.

Getting deprecation warnings for "add-path" and "set-env"
2 participants