Replace bundled conda-index with standalone conda-index package #4690

dholth · 2023-01-04T21:35:24Z

Description

Delegate conda-build's conda-index while keeping code for deprecation cycle reasons.

Could we delete the hotfix-source-repo argument as it has been unused for longer than a deprecation cycle?

We should probably delete the entire test_index.py which is now part of conda-index. Is there any part of index testing that needs to remain here, perhaps the legacy API? Assuming get_build_index is tested as part of all the conda-build invocations done by us.

Checklist - did you ...

Add a file to the news directory (using the template) for the next release's release notes?
Add / update necessary tests?
Add / update outdated documentation?

dholth · 2023-01-04T21:53:03Z

conda_build/build_index.py

+        "msys2-conda-epoch": "m2w64",
+    }
+    NAMESPACE_PACKAGE_NAMES = frozenset(NAMESPACES_MAP)
+    NAMESPACES = frozenset(NAMESPACES_MAP.values())


Is this used?

it wouldn't be surprising if conda.base.constants exists in all conda > 4.7 or the oldest one we can depend on.

dholth · 2023-01-04T21:53:33Z

conda_build/build_index.py

+# Copyright (C) 2014 Anaconda, Inc
+# SPDX-License-Identifier: BSD-3-Clause
+"""
+conda-build's use of conda-index, delegated to now-separate conda-index package.


Copied to make keeping track of all the changes easier, and to discourage future conda_build.index imports

To clarify, we couldn't continue to have index.py as the place for this code to live? Or asked differently, the get_build_index function, is that something that we have in conda-index? I'm a little worried we're introducing another place where users will end up importing from :)

get_build_index is only in conda-build. It is mostly about fetching repodata.json instead of generating repodata.json. It would of course be possible to keep it in conda_build.index

However we might want to raise deprecationwarning when conda_build.index is imported.

I don't think this submodule should exist.

Plus, all of conda.index should be deprecated with warnings to import from conda-index instead.

Do you mean conda.index or conda_build.index

Reminder that conda-build's get_build_index doesn't belong in conda-index.

@kenodegard @jezdez I moved this module back to conda_build.index

dholth · 2023-01-04T21:54:20Z

conda_build/cli/main_index.py

@@ -90,8 +90,8 @@ def execute(args):

    api.update_index(args.dir, check_md5=args.check_md5, channel_name=args.channel_name,


Would love to delete this whole file.

conda-index has a nice click CLI, but doesn't take the same arguments.

this might be able to use the non-api update_index, the api version is basically "call update_index for every dir in a list"

dholth · 2023-01-04T21:54:37Z

conda_build/environ.py

-                                      channel_urls=channel_urls, debug=debug, verbose=verbose,
-                                      locking=locking, timeout=timeout)
+    bldpkgs_dir = list(bldpkgs_dirs)[0]
+    index, index_ts, _ = build_index.get_build_index(subdir, bldpkgs_dir, output_folder, False,


inline function courtesy of pycharm refactor feature

conda_build/index.py

dholth · 2023-01-04T21:55:53Z

conda_build/index.py

-
+    return build_index.update_index(dir_path, check_md5, channel_name, patch_generator, threads,
+                 verbose, progress, hotfix_source_repo, subdirs, warn,
+                 current_index_versions, debug, index_file)


As of today an unused copy of the full index implementation remains in this file, for deprecation cycle reasons.

tests/test_index.py

jezdez

I have a few questions about the deprecation strategy of this code, and generally think we need to use deprecation warnings here and not only shims around the old code paths.

tests/test_index.py

jezdez · 2023-01-06T10:12:33Z

conda_build/build_index.py

+# Copyright (C) 2014 Anaconda, Inc
+# SPDX-License-Identifier: BSD-3-Clause
+"""
+conda-build's use of conda-index, delegated to now-separate conda-index package.


To clarify, we couldn't continue to have index.py as the place for this code to live? Or asked differently, the get_build_index function, is that something that we have in conda-index? I'm a little worried we're introducing another place where users will end up importing from :)

dholth · 2023-01-06T12:28:41Z

I'm still figuring out the best way to do deprecation. The current strategy is to leave the unused methods in, and come back a year later to remove the code. Deprecating the CLI is still an unanswered question. Some of the indexing has changed little enough that the old CLI could be kept in forever, minus the "list of files to re-index" feature.

.github/workflows/tests.yml

dholth · 2023-01-20T19:45:49Z

@kenodegard I updated, and tried to keep the main branch's version of the test configs.

dholth · 2023-02-03T15:41:16Z

I've updated this to remove the new build_index.py and copy it over to index.py instead (since the redundant fuctionality in index is in standalone conda-index).

kenodegard

Minor edits but is otherwise good to go

conda_build/build.py

conda_build/cli/main_index.py

kenodegard · 2023-02-24T00:42:13Z

tests/test_api_skeleton_cran.py

@@ -17,7 +17,7 @@
 @pytest.mark.parametrize(
    "package,license_id,license_family,license_files",
    [
-        ("r-rmarkdown", "GPL-3", "GPL3", {"GPL-3"}),
+        # ("r-rmarkdown", "GPL-3", "GPL3", {"GPL-3"}),  # fails with connection error


This is a flaky test, not a failing test

Seems like a pointless test IMO

That's fine but let's have that conversation/effort elsewhere, we want to keep PRs as focused as possible

We constantly face a simple choice between a focused PR and a PR with passing tests.

conda_build/index.py

news/4645-use-conda-index

kenodegard · 2023-02-24T01:09:04Z

news/4645-use-conda-index

+* `conda-build index --file` is no longer supported.
+* `conda-build index --hotfix-source-repo` is removed.


Suggested change

* `conda-build index --file` is no longer supported.

* `conda-build index --hotfix-source-repo` is removed.

* `conda-build index --file` is no longer supported and is pending deprecation. (#4690)

* `conda-build index --hotfix-source-repo` is no longer supported and is pending deprecation. (#4690)

Trying to communicate that --file no longer does anything, and --hotfix-source-repo produces an "unknown command line argument" error.

Per CEP9 we shouldn't be removing things outright. We should update the code to allow these arguments but raise deprecation warnings with instructions on what to do instead.

Doesn't CEP9 only apply to conda?

This script is probably run in a non-interactive cron job. We could print a warning and fail to do what the user expects, since they are not actually there to observe the deprecation warning. Instead the current code exits.

Co-authored-by: Ken Odegard <kodegard@anaconda.com>

kenodegard

My biggest question is how do we know if the new conda-index works like the old conda-index? Is it possible to get tests/test_index.py to verify the new codes? Or add tests/cli/test_main_index.py?

dholth · 2023-03-03T17:33:40Z

My biggest question is how do we know if the new conda-index works like the old conda-index? Is it possible to get tests/test_index.py to verify the new codes? Or add tests/cli/test_main_index.py?

conda-index has its own pretty good test suite that is based on the old conda-build suite, in the same way that the rest of the conda-index code is based on its counterpart from conda-build. It has also been in production for defaults & conda-forge for 6+ months.

dholth · 2023-03-03T18:07:00Z

This isn't complying with the conda deprecation policy. The index.py module has dramatically changed, and we haven't meticulously gone through to deprecate each individual changed function or provide an alternative; and keeping the unchanged old code around for a deprecation cycle while putting the new code in a separate module also appears to be unacceptable.

dholth added 2 commits January 4, 2023 15:12

depend on conda-index

3d54dfa

delegate indexing to conda-index package

653cb83

conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Jan 4, 2023

dholth self-assigned this Jan 4, 2023

dholth added the in-progress issue is actively being worked on label Jan 4, 2023

remove obsolute index_file test

5ef0f1f

dholth requested a review from kenodegard January 4, 2023 21:52

dholth commented Jan 4, 2023

View reviewed changes

dholth added 2 commits January 4, 2023 17:06

add news

71eab18

add conda-index to install_conda_build_test_deps

0596350

jezdez requested changes Jan 6, 2023

View reviewed changes

dholth added 5 commits January 6, 2023 09:06

Merge remote-tracking branch 'origin/main' into conda-index

272caf9

format conda_build/index.py for pre-commit

05d7f05

remove unnecessary import fallback

dd76ea6

linter changes

8eeaec6

update windows ci deps

f9fffbd

dholth commented Jan 6, 2023

View reviewed changes

dholth added 5 commits January 6, 2023 10:21

python-libarchive-c is required after all, for utils

c706da4

remove test_index (exists in standalone conda-index tests)

c1e57ef

deal with update_index(subdir) instead of update_index(dir)

e59b860

use single thread when indexing for build

ad6c0f7

Merge remote-tracking branch 'origin/main' into conda-index

d9cc516

dholth added 3 commits February 3, 2023 10:21

remove build_index module

560ad71

Merge remote-tracking branch 'origin/main' into conda-index

1e8f1d9

format environ.py

74b2d7c

dholth mentioned this pull request Feb 3, 2023

format environ.py with black #4765

Merged

add conda-index dependency to recipe

f047145

dholth added 3 commits February 3, 2023 11:19

add conda-index to requirements.txt

e27c8fb

restore subdirs fix

1eb6752

Merge remote-tracking branch 'origin/main' into conda-index

2e7c40c

dholth closed this Feb 4, 2023

dholth reopened this Feb 4, 2023

dholth added 7 commits February 4, 2023 10:06

ruamelize

cc63b28

adjust distutils patch deps

fe3647e

lint fixes

ad31a23

keyword arguments

8c0117b

indent

55e5ba2

Merge remote-tracking branch 'origin/main' into conda-index

51ba55a

remove package from test

a6cbec4

kenodegard requested changes Feb 24, 2023

View reviewed changes

Apply suggestions from code review

f1d52e2

Co-authored-by: Ken Odegard <kodegard@anaconda.com>

kenodegard reviewed Mar 3, 2023

View reviewed changes

dholth closed this Mar 3, 2023

github-actions bot added the locked [bot] locked due to inactivity label Mar 3, 2024

github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace bundled conda-index with standalone conda-index package #4690

Replace bundled conda-index with standalone conda-index package #4690

dholth commented Jan 4, 2023 •

edited

Loading

dholth Jan 4, 2023

dholth Jan 6, 2023

dholth Jan 4, 2023

jezdez Jan 6, 2023

dholth Jan 6, 2023 •

edited

Loading

dholth Jan 6, 2023

kenodegard Jan 24, 2023

dholth Jan 24, 2023

dholth Feb 3, 2023

dholth Jan 4, 2023

dholth Jan 6, 2023 •

edited

Loading

dholth Jan 4, 2023

dholth Jan 4, 2023

jezdez left a comment

jezdez Jan 6, 2023

dholth commented Jan 6, 2023 •

edited

Loading

dholth commented Jan 20, 2023

dholth commented Feb 3, 2023

kenodegard left a comment

kenodegard Feb 24, 2023

dholth Feb 24, 2023

kenodegard Mar 3, 2023

dholth Mar 3, 2023

kenodegard Feb 24, 2023

dholth Feb 24, 2023

kenodegard Mar 3, 2023

dholth Mar 3, 2023

dholth Mar 3, 2023

kenodegard left a comment

dholth commented Mar 3, 2023 •

edited

Loading

dholth commented Mar 3, 2023 •

edited

Loading

		@@ -90,8 +90,8 @@ def execute(args):

		api.update_index(args.dir, check_md5=args.check_md5, channel_name=args.channel_name,

		* `conda-build index --file` is no longer supported.
		* `conda-build index --hotfix-source-repo` is removed.

Replace bundled conda-index with standalone conda-index package #4690

Replace bundled conda-index with standalone conda-index package #4690

Conversation

dholth commented Jan 4, 2023 • edited Loading

Description

Checklist - did you ...

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dholth Jan 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dholth Jan 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jezdez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dholth commented Jan 6, 2023 • edited Loading

dholth commented Jan 20, 2023

dholth commented Feb 3, 2023

kenodegard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kenodegard left a comment

Choose a reason for hiding this comment

dholth commented Mar 3, 2023 • edited Loading

dholth commented Mar 3, 2023 • edited Loading

dholth commented Jan 4, 2023 •

edited

Loading

dholth Jan 6, 2023 •

edited

Loading

dholth Jan 6, 2023 •

edited

Loading

dholth commented Jan 6, 2023 •

edited

Loading

dholth commented Mar 3, 2023 •

edited

Loading

dholth commented Mar 3, 2023 •

edited

Loading