Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,22 @@
Changelog
=========

.. _`release:unreleased`:

Unreleased
----------

:Released: Unreleased

Bug fixes
.........

- πŸ› Sort need link and backlink lists in ``needs.json`` and HTML output using
natural, case-insensitive ordering (e.g. ``REQ_2`` < ``REQ_9`` < ``REQ_10``)
and collapse duplicate entries, so build outputs are reproducible regardless
of need load order (e.g. when using :ref:`needs_external_needs`)
(:issue:`1371`)

.. _`release:8.1.1`:

8.1.1
Expand Down
5 changes: 5 additions & 0 deletions sphinx_needs/directives/need.py
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,11 @@ def resolve_links(
message = f"Need '{need.id}' has unknown outgoing link '{need_link.to_filter_string()}' in field '{link_type}'"
_emit_link_warning(need, message, "link_outgoing")

# Sort link lists alphabetically so that outputs (needs.json, HTML) are
# deterministic and reproducible, regardless of needs/external_needs load order.
for need in needs.values():
need.sort_links()


def _emit_link_warning(need: NeedItem, message: str, subtype: WarningSubTypes) -> None:
"""Emit a warning for a link issue, using the appropriate location."""
Expand Down
41 changes: 41 additions & 0 deletions sphinx_needs/need_item.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
# For NeedItem, we allow mutability, but only for values, i.e. it should not allow adding or removing keys.
from __future__ import annotations

import re
from collections.abc import Iterable, Iterator, Mapping, Sequence
from dataclasses import dataclass, field
from itertools import chain
Expand Down Expand Up @@ -371,6 +372,27 @@ def to_link_string(self) -> str:
return f"{base}{open_b}{self.condition}{close_b}"


_NATURAL_SORT_RE = re.compile(r"(\d+)")
Comment thread
ubmarco marked this conversation as resolved.


def _natural_sort_key(value: str) -> list[str | int]:
"""Build a sort key for natural ordering: digit runs are compared as ints.

For example, ``REQ_2`` < ``REQ_9`` < ``REQ_10`` (instead of the default
lexicographic ``REQ_10`` < ``REQ_2`` < ``REQ_9``). Non-digit runs are
case-folded so the order is case-insensitive.
"""
return [
int(tok) if tok.isdigit() else tok.casefold()
for tok in _NATURAL_SORT_RE.split(value)
]


def _link_natural_sort_key(link: NeedLink) -> list[str | int]:
"""Natural sort key for a :class:`NeedLink`, based on its filter string."""
return _natural_sort_key(link.to_filter_string())


class NeedItem:
"""A class representing a single need item."""

Expand Down Expand Up @@ -883,6 +905,25 @@ def reset_backlinks(self) -> None:
for k in part.backlinks:
part.backlinks[k] = []

def sort_links(self) -> None:
Comment thread
ubmarco marked this conversation as resolved.
"""Sort all link and backlink lists in place, removing duplicates.

Sorts outgoing links, backlinks, and part backlinks by the link's
string representation, treating embedded digit sequences as integers
so that e.g. ``REQ_2`` < ``REQ_9`` < ``REQ_10``. This makes the order
of linked need IDs deterministic, regardless of the order in which
they were added or the iteration order of the needs collection.
Duplicate :class:`NeedLink` entries (same id, part, and condition)
are collapsed.
"""
for key, value in self._links.items():
self._links[key] = sorted(set(value), key=_link_natural_sort_key)
for key, value in self._backlinks.items():
self._backlinks[key] = sorted(set(value), key=_link_natural_sort_key)
for part in self._parts.values():
for key, value in part.backlinks.items():
part.backlinks[key] = sorted(set(value), key=_link_natural_sort_key)

def add_backlink(self, link_type: str, backlink: str | NeedLink) -> None:
"""Add a backlink to the need."""
if link_type not in self._backlinks:
Expand Down
5 changes: 2 additions & 3 deletions tests/__snapshots__/test_dynamic_functions.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@
'id': 'CON_SPEC_1',
'lineno': 13,
'links': list([
'CON-REQ-3',
'CON_REQ_1',
'CON_REQ_2',
'CON-REQ-3',
]),
'section_name': 'LINKS FROM CONTENT',
'sections': list([
Expand All @@ -109,9 +109,9 @@
'id': 'CON_SPEC_2',
'lineno': 23,
'links': list([
'CON-REQ-3',
'CON_REQ_1',
'CON_REQ_2',
'CON-REQ-3',
]),
'section_name': 'LINKS FROM CONTENT',
'sections': list([
Expand Down Expand Up @@ -180,7 +180,6 @@
'lineno': 51,
'links': list([
'CON_REQ_2',
'CON_REQ_2',
]),
'section_name': 'LINKS FROM CONTENT',
'sections': list([
Expand Down
2 changes: 1 addition & 1 deletion tests/__snapshots__/test_external.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -604,8 +604,8 @@
'id': 'SPEC_1',
'lineno': 12,
'links': list([
'REQ_1',
'EXT_TEST_01',
'REQ_1',
]),
'section_name': 'TEST DOCUMENT EXTERNAL',
'sections': list([
Expand Down
2 changes: 1 addition & 1 deletion tests/__snapshots__/test_field_defaults.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
'SPEC_3',
]),
'link2': list([
'SPEC_2',
'SPEC_1',
'SPEC_2',
]),
'link2_back': list([
'SPEC_1',
Expand Down
16 changes: 8 additions & 8 deletions tests/__snapshots__/test_link_conditions.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,15 @@
'id': 'REQ_001',
'lineno': 7,
'links_back': list([
'EXT_COND_PASS',
'EXT_COND_NONE',
'EXT_COND_PASS',
'IMP_COND_NONE',
'IMP_COND_PASS',
'SPEC_001',
'SPEC_003',
'SPEC_004',
'SPEC_005',
'SPEC_006',
'IMP_COND_PASS',
'IMP_COND_NONE',
]),
'section_name': 'Requirements',
'sections': list([
Expand All @@ -92,8 +92,8 @@
'lineno': 11,
'links_back': list([
'EXT_COND_FAIL',
'SPEC_002',
'IMP_COND_FAIL',
'SPEC_002',
]),
'section_name': 'Requirements',
'sections': list([
Expand Down Expand Up @@ -836,15 +836,15 @@
'id': 'REQ_001',
'lineno': 7,
'links_back': list([
'EXT_COND_PASS',
'EXT_COND_NONE',
'EXT_COND_PASS',
'IMP_COND_NONE',
'IMP_COND_PASS',
'SPEC_001',
'SPEC_003',
'SPEC_004',
'SPEC_005',
'SPEC_006',
'IMP_COND_PASS',
'IMP_COND_NONE',
]),
'section_name': 'Requirements',
'sections': list([
Expand All @@ -863,8 +863,8 @@
'lineno': 11,
'links_back': list([
'EXT_COND_FAIL',
'SPEC_002',
'IMP_COND_FAIL',
'SPEC_002',
]),
'section_name': 'Requirements',
'sections': list([
Expand Down
2 changes: 1 addition & 1 deletion tests/__snapshots__/test_list2need.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -426,8 +426,8 @@
'NEED-2',
]),
'links_back': list([
'NEED-Z',
'NEED-4',
'NEED-Z',
]),
'max_amount': None,
'max_content_lines': None,
Expand Down
2 changes: 1 addition & 1 deletion tests/__snapshots__/test_need_constraints.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@
'links': list([
]),
'links_back': list([
'SP_109F4',
'SP_3EBFA',
'SP_109F4',
]),
'max_amount': None,
'max_content_lines': None,
Expand Down
2 changes: 1 addition & 1 deletion tests/__snapshots__/test_needextend.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@
'lineno': 4,
'links': list([
'REQ_A_1',
'REQ_D_1',
'REQ_B_1',
'REQ_D_1',
]),
'links_back': list([
]),
Expand Down
10 changes: 5 additions & 5 deletions tests/__snapshots__/test_needimport.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@
'R_F4722',
]),
'links_back': list([
'filter_IMPL_01',
'IMPL_01',
'T_C3893',
'filter_IMPL_01',
]),
'section_name': 'TEST DOCUMENT IMPORT',
'sections': list([
Expand Down Expand Up @@ -315,8 +315,8 @@
'is_import': True,
'lineno': 4,
'links': list([
'OWN_ID_123',
'IMPL_01',
'OWN_ID_123',
]),
'parent_need': 'T_5CCAA',
'parent_needs': list([
Expand Down Expand Up @@ -679,8 +679,8 @@
'is_import': True,
'lineno': 16,
'links': list([
'collapsed_OWN_ID_123',
'collapsed_IMPL_01',
'collapsed_OWN_ID_123',
]),
'parent_need': 'collapsed_T_5CCAA',
'parent_needs': list([
Expand Down Expand Up @@ -1127,8 +1127,8 @@
'is_import': True,
'lineno': 9,
'links': list([
'hidden_OWN_ID_123',
'hidden_IMPL_01',
'hidden_OWN_ID_123',
]),
'parent_need': 'hidden_T_5CCAA',
'parent_needs': list([
Expand Down Expand Up @@ -1593,8 +1593,8 @@
'is_import': True,
'lineno': 23,
'links': list([
'test_OWN_ID_123',
'test_IMPL_01',
'test_OWN_ID_123',
]),
'parent_need': 'test_T_5CCAA',
'parent_needs': list([
Expand Down
Loading
Loading