Code Monkey home page Code Monkey logo

Comments (13)

reillysiemens avatar reillysiemens commented on May 24, 2024 1

@ThomasFWise: From the installer script's perspective, it's not aware that the operation is a downgrade or an upgrade. It's installing whatever version is requested. That's part of the

Avoid any SemVer comparison in the install scripts.

goal that @kyle-rader mentioned. Those comparisons are non-trivial, especially on Unix platforms where there isn't built-in JSON support for interacting with the GitHub API.

The installer actually doesn't even avoid installation if version 2 is installed. If you ask for version 2 and version 2 is already installed it blows away any previously existing installation and installs fresh, which it does to enable repairing a broken installation.

If what you're suggesting is that we do something like add a -UpdatePath flag and switch the default behavior of the installer to not update the $PATH at all I could see that. It would make the installation process less "dangerous" globally. However, it would also make the installer less useful for a fresh installation. The default then would be to install this tool and then have to go configure your $PATH manually.

Instead, with a -NoUpdatePath flag, if you know that you will happen to have multiple competing installations on your system and therefore $PATH updates won't be safe you can opt out of them and manually control the $PATH to your liking.

The danger of downgrading oneself in a multi-version should be addressed here by asserting control over the $PATH (which @kyle-rader touched on by mentioning that a monorepo environment could "prepend X to PATH"). A global install could change the user's $PATH, but it won't get rid of an existing installation, so as long as some environment knows which version it wants to point to it will be available. Our install script appends to $PATH, so it will take the least precedence over any per-environment prepending.

The proposal here is probably best summarized as

We want to stop providing the abstraction of latest because it's proven unreliable. We also want to provide a way to opt out of destructive operations (like $PATH changes) if you know those are unsafe in your environment.

There's a line between being an installer script and a package manager. I'd prefer to stay as much towards the installer script side of that balance. The goal is to get the bits to disk (and maybe make them quickly executable), not to manage versions.

from microsoft-authentication-cli.

reillysiemens avatar reillysiemens commented on May 24, 2024 1

The monorepo/non-monorepo divide is not a great way to talk about this, I agree. I was really just trying to describe a scenario where multiple versions of this tool are installed alongside one another. You're 💯% correct that there's only one $PATH.

Both append and prepend are correct in this context, @ThomasFWise. The installation script will only ever append. Because we append, any users managing their own environment are free to prepend, secure in the knowledge that their choice will always take precedence. Any external tooling (initialization scripts, etc.) can temporarily override the user's globally defined $PATH and point to whichever version of azureauth they expect to use in their specific context.

from microsoft-authentication-cli.

marcelais avatar marcelais commented on May 24, 2024 1

Yep, that sounds good.

from microsoft-authentication-cli.

ThomasFWise avatar ThomasFWise commented on May 24, 2024 1

That sounds reasonable.

from microsoft-authentication-cli.

reillysiemens avatar reillysiemens commented on May 24, 2024

Thanks for reporting this, @marcelais. Unfortunately, at the moment we don't have any great ideas for a solution that don't involve teaching the installation scripts how to understand SemVer, which we aren't keen on at the moment.

The most likely path forward will be for us to pursue proper package management for the various supported platforms (e.g. winget packages, .deb/.rpm packages, etc.), but we don't have an ETA for that just yet.

For now, I'd like to acknowledge this issue by leaving it open and invite any suggestions for a good cross-platform solution.

from microsoft-authentication-cli.

kyle-rader avatar kyle-rader commented on May 24, 2024

I think this bug is more important if we want to enable other applications to be able to install the CLI using the scripts. I was just thinking through using the scripts in a library wrapper, and we wouldn't want App 1 to install and use 0.3.0, another app installs 0.2.0 and reset latest back, at least internally this could confuse things.

from microsoft-authentication-cli.

kyle-rader avatar kyle-rader commented on May 24, 2024

@marcelais , @AzureAD/security-experience , @reillysiemens and I have talked through how to change the installation to avoid thrashing on the version latest is pointing to.

We think we want to achieve

  • The ability to install yourself, and have azureauth on the PATH.
  • The ability to install a version, and not update the PATH
  • Avoid any SemVer comparison in the install scripts.

Versioned installs only (no more latest):

Main logic:

  • clean install X (default): installs X, and replaces current version on PATH with X.
  • clean install X (no-path): install X.

Example Scenarios

Clean Install 3 (default)

  • install version 3.
  • add version 3 to PATH.

Clean Install 3 (no-path)

  • install version 3

Dirty Install 4 (default) (with 3 on PATH)

  • install version 4
  • replace version 3 with 4 in PATH

Dirty Install 4 (no-path) (with 3 on PATH)

  • install version 4

Dirty Install 2 (default) (with 3 on the PATH)

  • install 2
  • replace version 3 with 2 in PATH

Dirty Install 2 (no-path) (with 3 on PATH)

  • install 2

MonoRepo Scenario:

Clean Install version X (no-path)

  • install version X

MonoRepo

  • prepends X to PATH

from microsoft-authentication-cli.

ThomasFWise avatar ThomasFWise commented on May 24, 2024

I would prefer that an additional option be required to downgrade my path. Especially with as many things as may bundle an call to the installer, I really don't want a simple mistake by one author to downgrade everyone. It's really easy to do too:
Have version 3 installed. Tool runs and just checks
Is version 2 installed? No -> Install version 2
Rather than:
Is version 2 or greater installed? Yes -> Do nothing

There could be scenarios where downgrading the version on the path is correct, but I would prefer it require explicit request.

from microsoft-authentication-cli.

ThomasFWise avatar ThomasFWise commented on May 24, 2024

You say append, but the Monorepo example says prepend. If you are really appending to the path, then I would be ok with that. Prepend has the problems of potentially silently changing what version someone is using though, so I would be hesitant of that.

I'm not sure you can really say there is a monorepo/non-monorepo divide though. The PATH is not separate, so if I'm working on a project that uses this public tool in a non-monorepo configuration it has the potential to mess up the PATH for my other projects, or even my monorepo configured project.

from microsoft-authentication-cli.

marcelais avatar marcelais commented on May 24, 2024

One reason we used 'latest' was to avoid having to know the exact install path to AzureAuth. If the install path was predictable, we could get away with not using latest at all but using the version that was installed.

One challenge is that when you ask to install 'v0.3.0' the actual install directory is 'azureauth-v0.3.0-win10-x64' (on my Windows machine) so the installer would have to "know" that full extension. If this is a cross-platform script, then it would have to know all of the various platform flavours.

If the installer were to install to the folder that was named exactly the same as the version that was passed into the script, that would make predicting the path easier and also allow the script to more reliably determine if AzureAuth is already installed before calling the installer.

A similar option would be to have the install path configuration option be the exact path that we install to instead of a "root" directory that you create a versioned directory underneath. [Essentially, letting the calling script manage the version directory structure.]

from microsoft-authentication-cli.

kyle-rader avatar kyle-rader commented on May 24, 2024

@ThomasFWise are you satisfied by Tucker's last reply? I think we acknowledge the risks, that a user could install a new version, it updates their PATH, but in our primary monorepo scenario so far, our environment is not broken by this since it does/will prepend a specific version.

from microsoft-authentication-cli.

reillysiemens avatar reillysiemens commented on May 24, 2024

@marcelais: Your suggestion is completely reasonable (and preferable). In fact, when we started out, we used that approach and just named the directory according to its version. Unfortunately, in #49 we undid that as a hotfix for a bug that later turned out to be the result of file lock timing with DLLs for a running azureauth instance (see #99, #100, #102). Now that we taskkill any running azureauth instance before install we should hit that with much less frequency and it's likely safe to revert to a $install_root + $version directory scheme. That would allow us to get rid of latest (which has always been something of a lie) and still have a platform-agnostic install path.

Unfortunately, it's a backwards incompatible change. However, because we've only published 4 versions of azureauth so far we could modify the install script to just do an equality check on those 4 versions and retain the previous behavior (extract to azureauth-${version}-${runtime} and create/update latest). After enough time has elapsed (maybe 6 months or so) we can think about removing the extra logic from the installer.

So, if we accept that, the TL;DR on the proposed changes is:

  1. Update the install script to stop creating a latest directory (unless one of the 4 existing versions is requested).
  2. Update the install script to use platform-agnostic path (unless one of the 4 existing versions is requested).
  3. Add a -NoUpdatePath flag to help avoid modifications to the global $PATH.
  4. Encourage/require that multi-version environments pin the versions they're installing and prepend to the $PATH for any temporary overrides to a global installation.

How does that all sound, @kyle-rader, @marcelais, @ThomasFWise? 🤔

from microsoft-authentication-cli.

kyle-rader avatar kyle-rader commented on May 24, 2024

Closed with #120 - no more latest directory.

from microsoft-authentication-cli.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.