[mono][ios] System.Formats.Tar.Tests are failing with System.ArgumentException #88049

Activity

1. kotlarmilos

   added

   blocking-clean-ciBlocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms'Blocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms'

   area-Codegen-AOT-mono

   os-tvosApple tvOSApple tvOS

   Known Build ErrorUse this to report build issues in the .NET Helix tabUse this to report build issues in the .NET Helix tab

   on Jun 26, 2023
2. kotlarmilos

   added this to the 8.0.0 milestone on Jun 26, 2023
4. kotlarmilos commented on Jul 11, 2023

   

   kotlarmilos

   on Jul 11, 2023

   MemberAuthor

   More actions

   @carlossanlop It seems that the regression was introduced in #87534. The issue appears to be specific to the ios and tvos simulators.
5. kotlarmilos

   added

   area-System.Formats.Tar

   on Jul 11, 2023
7. kotlarmilos

   removed

   area-Codegen-AOT-mono

   on Jul 11, 2023
8. carlossanlop

   self-assigned this

   on Jul 13, 2023
9. kotlarmilos commented on Jul 24, 2023

   

   kotlarmilos

   on Jul 24, 2023

   Last edited by kotlarmilos

   MemberAuthor

   More actions

   This fails only on apple mobile simulators that run Mono Interpreter. My assumption is that the GetUtf8TextLength doesn't return the same value with Interpreter and AOT, but still have to confirm that locally.

   /cc: @BrzVlad

   const ushort limit = 100;
   ReadOnlySpan<char> name = _name;
   int encodedLength = GetUtf8TextLength(name);
   
   if (encodedLength > FieldLengths.Name)
   {
       if (_format is TarEntryFormat.V7)
       {
           throw new ArgumentException(SR.Format(SR.TarEntryFieldExceedsMaxLength, nameof(TarEntry.Name)), ArgNameEntry);
       }
   
       int utf16NameTruncatedLength = GetUtf16TruncatedTextLength(name, FieldLengths.Name);
       name = name.Slice(0, utf16NameTruncatedLength);
   }
10. kotlarmilos commented on Jul 31, 2023

    

    kotlarmilos

    on Jul 31, 2023

    MemberAuthor

    More actions

    This issue is also occurring on iOS/tvOS devices. For instance in: https://dev.azure.com/dnceng-public/public/_build/results?buildId=357141&view=results. The length of fields exceeds the values set in:

    runtime/src/libraries/System.Formats.Tar/src/System/Formats/Tar/FieldLengths.cs

    Line 9 in 75ecb1a

    private const ushort Path = 100;

    @carlossanlop Should we update the fields length to match the following examples?
    

    runtime/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.Base.cs

    Line 11 in 7b2716b

    public static IEnumerable<object[]> GetExactRootDirMatchCases()
11. carlossanlop commented on Aug 8, 2023

    

    carlossanlop

    on Aug 8, 2023

    Contributor

    More actions

    Hi - The Path length used for V7 should stay as is, it's what the spec indicates and it should be respected.

    The test could be modified so that the temporary path directory is created with a much shorter path. Would that work?

    👍React with 👍2Reacted by Milos Kotlar and Alexander Köplinger
12. steveisok commented on Aug 8, 2023

    

    steveisok

    on Aug 8, 2023

    Member

    More actions

    Hi - The Path length used for V7 should stay as is, it's what the spec indicates and it should be respected.

    The test could be modified so that the temporary path directory is created with a much shorter path. Would that work?

    Yes, definitely.
13. 3 remaining items

    Load more
14. build-analysis

    removed this from the 9.0.0 milestone on Nov 15, 2023
15. ghost added

    untriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner

    on Nov 15, 2023
16. carlossanlop

    added this to the 9.0.0 milestone on Dec 6, 2023
17. ghost removed

    untriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner

    on Dec 6, 2023
18. carlossanlop commented on Dec 6, 2023

    

    carlossanlop

    on Dec 6, 2023

    Contributor

    More actions

    @kotlarmilos @steveisok the folder used to store files needs to have a shorter path for the test to work (the failure is expected for this tar format). Can you please change it to a shorter directory for this platform?

    👍React with 👍1Reacted by Milos Kotlar
19. kotlarmilos

    self-assigned this

    on Dec 6, 2023
20. ghost added

    in-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged

    on Dec 7, 2023
21. ghost removed

    in-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged

    on Dec 11, 2023
22. ghost added

    in-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged

    on Dec 11, 2023
23. ghost removed

    in-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged

    on Dec 11, 2023
24. ghost added

    in-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged

    on Dec 11, 2023
25. ghost removed

    in-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is merged

    on Dec 11, 2023
26. kotlarmilos

    linked a pull request that will close this issue[release/7.0-staging] [tests] Disables tests on Apple mobile platforms #95861on Dec 11, 2023
27. kotlarmilos

    closed this as completedon Dec 13, 2023
28. github-actions

    locked and limited conversation to collaborators on Jan 13, 2024