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

Add a node local scheme without dirty information? #962

Open
pmeier opened this issue Oct 20, 2023 · 7 comments
Open

Add a node local scheme without dirty information? #962

pmeier opened this issue Oct 20, 2023 · 7 comments

Comments

@pmeier
Copy link

pmeier commented Oct 20, 2023

Currently there are only options that either include the information of a dirty workdir either directly (dirty-tag) or indirectly through time (node-and-date / node-and-timestamp). The only other option is to turn off the local tag completely with no-local-version.

I would like to have a node scheme that includes the information of the current commit, but ignores any changes in the workdir.

Happy to send a PR if this proposal is accepted.

@RonnyPfannschmidt
Copy link
Contributor

What's the use case you wan@t to support with that

So far I intentionally left this out as it protects people from bad mistakes in the release process

Without a good use case I'd rather not include the feature

@pmeier
Copy link
Author

pmeier commented Oct 23, 2023

We are building documentation on readthedocs.org. This build injects some extra files and some additional configuration in existing files. I could potentially ignore the former, but can't really do anything about the latter.

As a result, all of the builds come out as dirty, e.g. https://ragna--101.org.readthedocs.build/en/101/references/rest-api/

image

So far I intentionally left this out as it protects people from bad mistakes in the release process

Could you elaborate on that. What does not having a dirty information protect users from? And how does that relate to the fact that there is the no-local-version scheme?

@RonnyPfannschmidt
Copy link
Contributor

It's actually a quite common occurrence that people starting to ramp up project management create setups where whats released is not whats commited

Those mistakes in ci are commonly missed unless it triggers visible state

I'm curious why your RTD build actually needs to create a dirty worktree, chances are it doesn't

As for the no local scheme, it's quite a old contribution that I do regret, I should properly deprecate it

@pmeier
Copy link
Author

pmeier commented Oct 23, 2023

I'm curious why your RTD build actually needs to create a dirty worktree, chances are it doesn't

I haven't looked into the actual changes, but rather on the files that were touched:

  • The build touched the conda environment file that we use to setup our requirements
  • The build touched the mkdocs.yml configuration file
  • The build inserted a readthedocs-data.js into our docs directory

As for the no local scheme, it's quite a old contribution that I do regret, I should properly deprecate it

Fair enough. With that context, my suggestion does not make a lot of sense.

@RonnyPfannschmidt
Copy link
Contributor

I recall that mkdocs specifically supports configuration Imports for having overrides on builds

I'm unfamiliar with conda, but it sounds like your setup does more than necessary

As for the js file, it sounds like it should either be pinned,or git ignored

@pmeier
Copy link
Author

pmeier commented Feb 1, 2024

Sorry for taking forever to get back here. I finally had a closer look into what is happening.

  1. It injects some slight modifications into both the environment.yml and mkdocs.yml.
    • In the environment.yml it adds a mkdocs requirement although we already have it in there. This could potentially be fixed by RTD.

    • In mkdocs it injects the following configuration options:

      extra_css:
        - /_/static/css/badge_only.css
        - /_/static/css/readthedocs-doc-embed.css
      extra_javascript:
        - readthedocs-data.js
        - /_/static/core/js/readthedocs-doc-embed.js
        - /_/static/javascript/readthedocs-analytics.js
      google_analytics: null
  2. In the process of editing these files, they get completely re-ordered. This makes no functional difference, but of course this is a change in the file that triggers the version to be dirty.

Assuming RTDs process is smart enough to not touch a file if it is already properly configured, the only thing we could do here is put these options into our regular configuration file and make the linked files dummies. But I don't think the assumption will hold given the environment.yml handling.

I guess our best bet is to find a pre-install hook and set the SETUPTOOLS_SCM_PRETEND_VERSION end var. I'll report back if that is possible.

@pmeier
Copy link
Author

pmeier commented Feb 1, 2024

RTD has some hooks like pre_install, but unfortunately

Each command is executed in a new shell process, so modifications done to the shell environment do not persist between commands

Meaning, setting SETUPTOOLS_SCM_PRETEND_VERSION will have no effect.

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

No branches or pull requests

2 participants