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

[Bug] Default configuration is used when config file specified via -config not found #2848

Closed
beatcracker opened this issue Sep 14, 2021 · 6 comments · Fixed by #3408
Closed

Comments

@beatcracker
Copy link

beatcracker commented Sep 14, 2021

Describe the bug

When invalid file name passed to the -config switch (missing file, etc...) no error message is shown and GitVersion silently assumes default configuration.

Expected Behavior

If -config switch is used and specified file not found an error message should be shown.

Actual Behavior

GitVersion silently assumes default configuration. This breaks the principle of least astonishment and makes it hard to debug issues with configuration, especially in CI.

Steps to Reproduce

The GitVersion will generate version using default configuration if the specified config file doesn't exist.

mkdir ./repro
cd ./repro
git init
dotnet tool install --global GitVersion.Tool
dotnet-gitversion -config foobar.xyz

Your Environment

  • Operating System and version: Windows 10
  • GitVesion: 5.7.0+Branch.main.Sha.8d177c6d666c8eeb1c6a6a2c71fd4b78741137d2

I think similar issue related to config file handling was raised already, but unfortunely I can't find it, since config + everything that I was able to come up with are too generic terms for GitHub issue search.

@asbjornu
Copy link
Member

I agree GitVersion should fail, but this can be considered a breaking change and should probably be postponed to v6?

@beatcracker
Copy link
Author

beatcracker commented Sep 21, 2021

Well, there are probably people out there who rely on this behavior (Hyrum's Law), but I'm not sure if keeping it worth the trouble.

For instance, I'm currently trying to debug why GitVersion refuses to read my config file specified via -config switch and I had to put a GitVersion.yml with garbage inside it into the repo root so GitVersion would actually fail and I could see that my custom config is not being used. And this happens locally. In CI that would be much bigger PITA, since with modern CI systems you can't always repo this on your machine.

So I can think of several improvements/fixes for this:

  • Breaking change: fail if specified config not found
  • Minor change: introduce additional flag that would enable fail behavior.
  • QoL improvement: log full config name to the console output

@HHobeck
Copy link
Contributor

HHobeck commented Jan 4, 2023

When invalid file name passed to the -config switch (missing file, etc...) no error message is shown and GitVersion silently assumes default configuration.

I think this statement is confusing a little bit and is not correct: GitVersion always takes the default GitVersion configuration in code. As a user you are able to override this configuration via parameter or defining the configuration file (values will be merged) but the base is still the default configuration in our case the gitflow workflow.

@HHobeck
Copy link
Contributor

HHobeck commented Jan 4, 2023

Well, there are probably people out there who rely on this behavior (Hyrum's Law), but I'm not sure if keeping it worth the trouble.

In my opinion it makes no sense to ensure an undocumented feature/behaviour (at least for the next major version) because it was there in the past. Defining a configuration file which is not existing should result in a error for sure.

@asbjornu
Copy link
Member

Sure, but we are also trying to close down the development of v5 to focus on v6. So unless it's very important, such as a security fix or similar, we would prefer to target everything for v6 from now on.

@arturcic
Copy link
Member

arturcic commented Mar 2, 2023

🎉 This issue has been resolved in version 6.0.0-beta.1 🎉
The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants